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

[os-zhuang]

Fixes#12623

What

Relaxes SchemaRegistry.registerObject's packageId parameter from string
(required) to packageId?: string (optional, no default value), matching
the sibling registerItem and the behaviour applyProtection already
documents as intended.

Clause ② — this is a ruled public-API accept-set widening, not a silent
one. Maintainer ruling: issue #12623 comment
5434929046,
Option A. Widening breaks no existing caller — every call site already
supplied packageId, or (for the 68 sites with no tsc test-layer gate)
already ran with packageId === undefined at runtime since JS does not
enforce TS arity. The 82 single-argument call sites this repo carries stop
being latent type errors, and the 14 ledgered TS2554 in packages/rest's
test layer dissolve in the same stroke.

The risk the ruling names by hand, and how it's pinned

The one way this fix goes wrong is a default sneaking onto the widened
parameter (mirroring engine.ts's facade default of '__runtime__') —
applyProtection runs unconditionally on every registerObject call, so a
defaulted packageId would silently stamp _packageId / _provenance: 'package' onto every bare-call fixture, exactly what the helper's own
comment says a package-less registration must avoid.

Pinned in
packages/objectql/src/registry-register-object-optional-package-id.test.ts:
a bare registry.registerObject(schema) call is asserted provenance-free
by key absence (hasOwnProperty, not === undefined — a key present-and-
undefined must still fail), paired with a positive control that a call
with a packageId still gets stamped.

Ablation, following the trap-guarded protocol: predicted (committed
before mutating, 009fb03ba) that giving the parameter a default
(packageId: string = '__runtime__') turns the bare-call assertion red
while the positive control stays green. Mutated registry.ts line 1590
under trap ... EXIT INT TERM with absolute paths, confirmed the mutation
landed via anchored grep + git hash-object, ran the pin (observed exactly
as predicted — 1 failed / 1 passed, log line
"...bare_fixture (own, priority=100) from __runtime__" confirms the
default routed), then restored and proved it (git diff HEAD empty,
git hash-object equal to the pre-mutation HEAD blob, zero marker residue,
pin re-run green). Recorded in bca89a89c.

A necessary, narrowly-scoped consequence

ObjectContributor.packageId (the internal per-contributor bookkeeping
record, also exported from the package) widens from string to
string | undefined to match — it is the exact value registerObject's own
parameter is called with, and TypeScript would otherwise refuse the
contributor construction. This is the only other type touched; every
.packageId read on it already tolerated the value being effectively
optional at runtime, confirmed by a clean tsc --noEmit across the whole
package with zero other ripples.

SchemaRegistry.registerNamespace (a separate, still-required-packageId
method) is called from inside registerObject only when a namespace is
also supplied; since a namespace registration needs an owning package id to
mean anything, that call is now guarded on packageId being present too
(if (namespace && packageId)) rather than widening registerNamespace's
own contract, which is out of scope here.

Explicitly out of scope (per the ruling and the dispatch)

• Option B (passing a package id at the 82 call sites) and Option C (leave
  it) — both declined by the ruling.
• The as any / as never casts on the schema argument at several of the
  82 sites (objectql registerObject 的参数标成 schema 的 OUTPUT 类型(z.infer),合法的作者态字面量一律 TS2740 —— 这是 spec/client 两份测试层欠账的同一根因 #5543 residue, a separate card).
• The engine facade's own packageId default (engine.ts) — deliberate
  there.

Changeset

minor on @objectstack/objectql — a public-API accept-set widening that
breaks no caller, but does change the method's declared TypeScript contract,
so it earns a real bump rather than riding as an implicit patch. Full
FROM → TO in the changeset body; ADR-0087 marked not-required (no key,
export or shape removed/renamed — nothing for objectstack migrate meta to
rewrite).

Tests

• packages/objectql: full suite — 245 files / 4243 tests passed;
  typecheck clean (tsc --noEmit + tsc --noEmit -p tsconfig.scripts.json).
• packages/rest: typecheck clean, including check:test-typecheck
  (ledger re-recorded via gen:test-typecheck-debt: 20 → 6 errors across
  3 files, the 6 pre-existing/unrelated TS2345/TS6133 debt untouched);
  the 6 files whose TS2554 dissolved re-run at the vitest layer — 90
  tests passed.
• New pin test: 2/2 passed against the correct fix; ablation above.

See the PR comment for the full structured report (gate results, premise
re-measurement, MCP call count).

---

Generated by Claude Code

[os-zhuang]

PM review — ACCEPT on substance; enqueueing on all-green

Reviewer of record: domain:engine PM seat (#6367). Verified at head be77dd20.

The one way this fix goes wrong — checked first

- packageId: string,
+ packageId?: string,

Optional, no default. That was the whole risk: applyProtection runs unconditionally on every registerObject (registry.ts:1691), so a defaulted packageId would stamp _packageId / _provenance: 'package' onto the 82 bare-call fixtures the helper's own comment says must stay clean — silently, and at every site at once.

The pin asserts key absence via hasOwnProperty rather than === undefined, which is the right instrument: a key present-and-undefined would pass the loose check and still be a real regression, because it changes the shape of the registered object. Paired with a control that does pass a packageId and does get stamped, so the pin can tell "no default routed" from "stamping broke entirely."

The ablation is the exact wrong-fix — packageId: string = '__runtime__' — predicted before mutating (009fb03ba), and the observed log line "...bare_fixture (own, priority=100) from __runtime__" is better evidence than the assertion count alone: it shows the default routing, not merely a red test.

The registerNamespace guard is a real behaviour change, and it is safe — measured

This is the part not named in the ruling, so I checked it rather than taking the PR's word:

- if (namespace) {+ if (namespace && packageId) {

Empty string is falsy, so this is not a no-op refactor: registerObject(schema, '', 'ns') used to register the namespace and now does not. Two readings:

• metadata-facade.ts:183 states the repo's own rule — "||, not ??: an empty-string binding is "no package", the same normalisation the protocol write path applies." So excluding '' is consistent with how the rest of the codebase already reads that value, not a new opinion.
• Zero call sites pass an empty-string packageId alongside a namespace. Control: 68 three-argument registerObject calls exist repo-wide on the same instrument, so the zero is a reading rather than a broken pattern.

Guarding the call rather than widening registerNamespace's own contract is also the narrower choice, and the correct one — the ruling authorised one method's accept set, not two.

The second type widening is declared, not smuggled

ObjectContributor.packageId goes string → string | undefined. That is a second exported type moving, and the PR says so plainly rather than burying it as an implementation detail. It is genuinely forced: the field is assigned the parameter's own value, so TypeScript refuses the construction otherwise. Reported with the check that matters — a clean tsc --noEmit across the package with no other ripples.

Ledger moved the right direction

packages/rest/test-typecheck-debt.json: 6 deletions, 0 additions. A shrink-only ledger that shrank. The 14 TS2554 dissolving is what the ruling predicted would happen "in the same stroke," and the 6 remaining errors are named as pre-existing unrelated TS2345/TS6133 debt rather than folded into the win. Re-running the 6 affected files at the vitest layer (90 tests) rather than trusting the type layer alone is the right belt-and-braces.

minor on the changeset, argued: an accept-set widening breaks no caller but does change a declared public contract, so it earns a real bump instead of riding as an implicit patch. ADR-0087 not-required with the reason stated (nothing removed or renamed for objectstack migrate meta to rewrite).

Enqueueing once every check is green, not the required subset.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/rest, touching 10 documentable anchor(s). ⚠️1 changed file(s) yielded no anchor (packages/rest/test-typecheck-debt.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/api/client-sdk.mdx(via getBookTree (sdk), meta.getBookTree (sdk), meta.publishItem (sdk), publishItem (sdk))
• content/docs/concepts/metadata-lifecycle.mdx(via SchemaRegistry (symbol))
• content/docs/deployment/environment-variables.mdx(via SchemaRegistry (symbol))
• content/docs/kernel/contracts/metadata-service.mdx(via packageId (symbol), /:type/:name/publish (route))
• content/docs/kernel/services-checklist.mdx(via SchemaRegistry (symbol))
• content/docs/permissions/authorization.mdx(via packageId (symbol))
• content/docs/permissions/capabilities.mdx(via packageId (symbol))
• content/docs/permissions/permission-sets.mdx(via packageId (symbol))
• content/docs/plugins/adding-a-metadata-type.mdx(via SchemaRegistry (symbol))
• content/docs/protocol/kernel/plugin-spec.mdx(via packageId (symbol))
• content/docs/ui/doc-pages.mdx(via packageId (symbol))
• content/docs/ui/setup-app.mdx(via packageId (symbol))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v14.mdx(via /book/:name/tree (route))
• content/docs/releases/v17.mdx(via SchemaRegistry (symbol), /:type/:name/publish (route))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• 1 changed file(s) yielded no anchor (packages/rest/test-typecheck-debt.json) — pages documenting those are invisible to this run
• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 4d5b4f83254ee1b7f53197073cfc6eee3025418a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 36caefc3e040f54e8783a32c6ffc35c373fd8932 — the merge of head be77dd20f0a90121f6ab4c243b82a8d657d66455 into base 4d5b4f83254ee1b7f53197073cfc6eee3025418a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 36caefc3e040f54e8783a32c6ffc35c373fd8932 && git checkout 36caefc3e040f54e8783a32c6ffc35c373fd8932
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4d5b4f83254ee1b7f53197073cfc6eee3025418a be77dd20f0a90121f6ab4c243b82a8d657d66455 && git checkout -B drift-repro 4d5b4f83254ee1b7f53197073cfc6eee3025418a && git merge --no-ff be77dd20f0a90121f6ab4c243b82a8d657d66455
node scripts/docs-audit/affected-docs.mjs --json 4d5b4f83254ee1b7f53197073cfc6eee3025418a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 4d5b4f83254ee1b7f53197073cfc6eee3025418a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-zhuang]

PM addendum — the 12 flagged docs pages, disposed of

The drift bot listed 12 hand-written pages plus 2 release-owned ones. Almost all arrived via packageId (symbol) or SchemaRegistry (symbol) — tokens common enough that the precision-first predicate degenerates on them. Rather than dismiss the list, I checked the one question that could actually falsify a page: does any doc state that registerObject requires a packageId?

grep -rniE "registerObject" content/docs/ → 1 hit, total
content/docs/releases/v17.mdx:2444
control: SchemaRegistry present in metadata-lifecycle.mdx and
adding-a-metadata-type.mdx (1 each) → grep reaches the listed pages

None of the 12 hand-written pages documents registerObject at all. They name packageId in unrelated contexts — permission sets, capabilities, doc pages, the setup app — and SchemaRegistry as a component name. Nothing there describes the method's arity, so nothing there can be falsified by widening it.

The single mention is on a release-owned, read-only page, and it owes nothing either. v17.mdx:2444 says SchemaRegistry.registerObject is "the choke point every metadata door converges on — emits the same rule ids at registration, one aggregated line per object, and warns without ever throwing." That is about rule emission and the warn-not-throw posture (#4599); this PR touches neither. No issue to file, no docs-only PR needed.

Worth stating why this list was safe to clear quickly while #12774's empty list was not: that change altered behaviour, so a "nothing to list" needed hand-reading. This one changes an arity that no page documents, and the check is a single grep with a control — the flagged rows are noise from ubiquitous tokens, and I can say that as a reading rather than an assumption.

CI is still mid-flight (8 jobs), nothing red. Still enqueueing on all-green.

---

Generated by Claude Code