Skip to content

Commit 0d24078

Browse files
os-zhuangclaude
andauthored
fix(objectql): wantOwner becomes an allow-list; inject owning_business_unit_id (ADR-0117 D1) (#5677) (#5802)
`applySystemFields` decided owner injection with a DENY-list — only 'org' and 'none' opted out — so ANY fourth `ownership` value fell through to the default branch and was stamped with `owner_id`. ADR-0117 D1's new `business_unit` tier means the opposite ("owned by a UNIT, not a person"): owner_id no, owning_business_unit_id yes. #4611's one-shot probe measured the inversion. The judgement is now a positive list (`undefined | 'user'`), which is behaviour-identical for the three values that exist today and stops a new tier from inheriting the owner branch by accident. A second anchor covers the union of the owner tiers with `business_unit` and injects `owning_business_unit_id` (lookup to sys_business_unit). The column is shaped after `organization_id` (server-stamped scope anchor), not after `owner_id` (user-assignable field): readonly, hidden, nullable, unindexed. None of those presumes ADR-0117 D2's undecided stamping policy — they grant no capability, so every D2 outcome stays reachable; the column is provisioned but inert until the stamping middleware (D2/D4) and backfill (D8) land. The objectql conformance test migrates the name from the documented-reserved group to Group A, where it is derived from the live injection code — the migration #4611 left a note for. Scope is the issue's work items 1-3. The spec-side JSDoc/description sync is #5767 and the `ownership` enum member is #5678, both strictly after this PR. Fixes #5677 Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx Co-authored-by: Claude <noreply@anthropic.com>
1 parent d97f2a2 commit 0d24078

4 files changed

Lines changed: 285 additions & 20 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): `wantOwner` 由排除式改为正面清单,并注入 `owning_business_unit_id`(ADR-0117 D1,#5677)
6+
7+
`applySystemFields` 判定是否注入 `owner_id` 的 `wantOwner` 原本是**排除式**的
8+
(`ownership !== 'org' && ownership !== 'none'`):只有两个值被排除,任何第四个
9+
`ownership` 值都会掉进默认分支、照常被盖上 `owner_id`。这与 ADR-0117 D1 的表格
10+
**恰好相反**——新增的 `business_unit` 档的全部含义就是「归属于组织单元而非个人」
11+
(`owner_id` ❌、`owning_business_unit_id` ✅)。#4611 的一次性探针已实测确认过这个
12+
反向结果。
13+
14+
**翻面。** 判定改为正面清单 `ownership === undefined || ownership === 'user'`
15+
(`managedBy` 平台表与 `sys_` 命名空间的跳过规则不变)。对今天存在的三个值
16+
**行为完全不变**:`undefined`/`user` 照常注入,`org`/`none` 照常排除;唯一的变化是
17+
新档位不再靠「落进默认分支」继承 owner 语义。这是先决单点:先翻面,枚举扩展
18+
(#5678)才是安全的。
19+
20+
**注入 `owning_business_unit_id`。** 记录级组织单元归属(lookup → `sys_business_unit`),
21+
按 D1 表格覆盖 `undefined`/`user`/`business_unit` 三档,`org`/`none` 不注入。
22+
23+
| `ownership` | `owner_id` | `owning_business_unit_id` |
24+
|---|---|---|
25+
| `undefined` / `user` | ✅ | ✅ |
26+
| `business_unit` | ❌ | ✅ |
27+
| `org` / `none` | ❌ | ❌ |
28+
29+
**用户可见的行为变化**:声明 `ownership: 'business_unit'` 的对象此前会被误注入
30+
`owner_id`,现在改为注入 `owning_business_unit_id`。(该值今天仍被 `ObjectSchema`
31+
的枚举拒收 —— 枚举扩展在 #5678,严格后置于本单;所以本次发布中这条路径只有
32+
引擎侧就绪,尚无法从元数据声明触达。)
33+
34+
列的形态比照 `organization_id`(服务端盖章的作用域锚点)而非 `owner_id`(用户可指派
35+
的业务字段):`readonly: true` + `hidden: true` + `required: false`,不建索引。三者
36+
都**不预设** ADR-0117 D2 尚未裁定的盖章策略(`pinned`/`follow_owner`/`transferable`)
37+
——它们不授予任何能力,因此 D2 的每种结论都仍然可达;盖章中间件(D2/D4)与回填
38+
(D8)落地前,该列被提供但处于惰性状态(恒为 NULL)。
39+
40+
`packages/spec` 未改动:`SystemFieldName.OWNING_BUSINESS_UNIT_ID` 的 JSDoc 与
41+
`object.zod.ts` 的描述同步在 #5767。该名早已在公开表单 server-managed 拒收名单上
42+
(#4611 提前登记),因此新列从第一天起就不可由匿名面客户端提供。

‎packages/objectql/src/registry.test.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,122 @@ describe('applySystemFields', () => {
732732
expect(out.fields.owner_id.readonly).toBe(true);
733733
});
734734

735+
// ── ADR-0117 D1 — owning_business_unit_id + the wantOwner allow-list ──
736+
//
737+
// The flip under test: `wantOwner` used to be a DENY-list
738+
// (`ownership !== 'org' && ownership !== 'none' && …`), so any value
739+
// outside those two — including D1's new `business_unit` tier — fell into
740+
// the default branch and got stamped with `owner_id`, the exact INVERSE of
741+
// D1's table. It is now an allow-list (`undefined | 'user'`), and a second
742+
// anchor covers the union of the owner tiers with `business_unit`:
743+
//
744+
// ownership owner_id owning_business_unit_id
745+
// undefined/user ✅ ✅
746+
// business_unit ❌ ✅
747+
// org ❌ ❌
748+
// none ❌ ❌
749+
//
750+
// ⚠️ `ownership: 'business_unit'` is NOT a legal ObjectSchema value yet —
751+
// the spec enum gains it in #5678, strictly AFTER this PR (that ordering is
752+
// #5677's whole point: the engine must honour the tier before the schema
753+
// can emit it, or the tier's first appearance gets the inverse result).
754+
// These fixtures therefore duck-type the schema, exactly as every other
755+
// opt-out case in this suite already does — `applySystemFields` takes a
756+
// `ServiceObject`, and the registry reads `ownership` as a value, not as a
757+
// Zod-parsed enum. When #5678 lands, the `as any` here can be dropped
758+
// without changing a single assertion.
759+
describe('[ADR-0117 D1] owning_business_unit_id injection', () => {
760+
it("does NOT inject owner_id for ownership: 'business_unit' — but DOES inject owning_business_unit_id", () => {
761+
// THE regression this issue exists to prevent. Under the old
762+
// deny-list this object was stamped `owner_id` (a person) even
763+
// though the tier's entire meaning is "owned by a unit, not a
764+
// person" — see #4611's one-shot probe.
765+
const unitOwned: any = { ...baseLead, name: 'inventory_item', ownership: 'business_unit' };
766+
const out = applySystemFields(unitOwned, { multiTenant: false });
767+
768+
expect(out.fields.owner_id).toBeUndefined();
769+
expect(out.fields.owning_business_unit_id).toMatchObject({
770+
type: 'lookup',
771+
reference: 'sys_business_unit',
772+
system: true,
773+
});
774+
// Tenant + audit columns are orthogonal to the ownership axis.
775+
expect(out.fields.organization_id).toBeDefined();
776+
expect(out.fields.created_at).toBeDefined();
777+
});
778+
779+
it("injects BOTH anchors on the default tier and on an explicit ownership: 'user'", () => {
780+
for (const schema of [baseLead, { ...baseLead, ownership: 'user' } as any]) {
781+
const out = applySystemFields(schema, { multiTenant: false });
782+
expect(out.fields.owner_id).toBeDefined();
783+
expect(out.fields.owning_business_unit_id).toBeDefined();
784+
}
785+
});
786+
787+
it("injects NEITHER anchor for ownership: 'org' / 'none'", () => {
788+
// The BU column follows `owner_id` out the door on the opt-out
789+
// tiers: a catalog/junction table has no per-record owner of
790+
// EITHER kind. (The pre-existing owner_id half of this is pinned
791+
// separately above — that test is what proves the allow-list flip
792+
// left the three existing values untouched.)
793+
for (const ownership of ['org', 'none'] as const) {
794+
const opted: any = { ...baseLead, ownership };
795+
const out = applySystemFields(opted, { multiTenant: false });
796+
expect(out.fields.owner_id).toBeUndefined();
797+
expect(out.fields.owning_business_unit_id).toBeUndefined();
798+
}
799+
});
800+
801+
it('does NOT inject owning_business_unit_id for managedBy / sys_* tables', () => {
802+
const platform: any = { name: 'proj_thing', managedBy: 'platform', fields: { msg: { type: 'text' } } };
803+
expect(applySystemFields(platform, { multiTenant: false }).fields.owning_business_unit_id).toBeUndefined();
804+
805+
const sysish: any = { name: 'sys_widget', fields: { msg: { type: 'text' } } };
806+
expect(applySystemFields(sysish, { multiTenant: false }).fields.owning_business_unit_id).toBeUndefined();
807+
808+
// …and the skip is not a side effect of the ownership tier: even
809+
// the tier that WANTS the column doesn't get it on a managed table.
810+
const managedUnitOwned: any = {
811+
name: 'proj_thing', managedBy: 'platform', ownership: 'business_unit', fields: { msg: { type: 'text' } },
812+
};
813+
expect(applySystemFields(managedUnitOwned, { multiTenant: false }).fields.owning_business_unit_id).toBeUndefined();
814+
});
815+
816+
it('is shaped like organization_id (server-stamped anchor), not like owner_id (assignable field)', () => {
817+
// The distinction is load-bearing, not cosmetic: D3 requires the
818+
// value to be derived and validated server-side and D4 makes
819+
// reassignment a transfer-class operation — neither guard has
820+
// landed, so no client write path may reach the column. `readonly`
821+
// + `hidden` grant no capability, which is what keeps every
822+
// undecided D2 policy (pinned / follow_owner / transferable)
823+
// reachable from here.
824+
const out = applySystemFields(baseLead, { multiTenant: true });
825+
const bu = out.fields.owning_business_unit_id;
826+
827+
expect(bu.readonly).toBe(true); // ≠ owner_id, which is reassignable
828+
expect(out.fields.owner_id.readonly).toBe(false);
829+
expect(bu.hidden).toBe(true);
830+
expect(bu.system).toBe(true);
831+
expect(bu.required).toBe(false); // nullable — nothing stamps it yet
832+
// No index: the hierarchical predicate that would use one ships
833+
// with the enterprise scope resolver (D6). Unlike organization_id,
834+
// it is not gated on multiTenant — it is simply absent.
835+
expect(bu.indexed).toBeUndefined();
836+
});
837+
838+
it('does NOT overwrite an author-declared owning_business_unit_id', () => {
839+
// Same precedence rule as every other injected column: `additions`
840+
// lose to `schema.fields`.
841+
const declared: any = {
842+
name: 'lead',
843+
fields: { owning_business_unit_id: { type: 'lookup', reference: 'sys_business_unit', label: 'Dept', readonly: false } },
844+
};
845+
const out = applySystemFields(declared, { multiTenant: false });
846+
expect(out.fields.owning_business_unit_id.label).toBe('Dept');
847+
expect(out.fields.owning_business_unit_id.readonly).toBe(false);
848+
});
849+
});
850+
735851
it('SchemaRegistry({ multiTenant: true }) auto-injects on registerObject', () => {
736852
const reg = new SchemaRegistry({ multiTenant: true });
737853
reg.registerObject({ name: 'lead', fields: { first_name: { type: 'text' } } } as any, 'crm', 'crm', 'own');

‎packages/objectql/src/registry.ts‎

Lines changed: 103 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveCrudAffordances, isTenancyDisabled, LEGACY_API_METHODS, AUDIT_PROVENANCE_FIELDS, type AuditProvenanceField } from '@objectstack/spec/data';
4+
import { SystemFieldName } from '@objectstack/spec/system';
45
import { resolveTenancyPosture, resolveSearchPinyinEnabled } from '@objectstack/types';
56
import { postureEnforcesWall } from '@objectstack/spec/security';
67
import { provisionSearchCompanion } from './search-companion.js';
@@ -233,6 +234,17 @@ export interface SchemaRegistryOptions {
233234
* opt-out is harmless (a spare nullable column), whereas forgetting to
234235
* ADD ownership — the failure mode we are eliminating — silently breaks
235236
* every owner-keyed feature.
237+
* - `owning_business_unit_id` — [ADR-0117 D1] record-level ORG-UNIT
238+
* ownership (lookup to `sys_business_unit`): the tier between `owner_id`
239+
* (a person) and `organization_id` (a tenant). Injected on the same
240+
* objects `owner_id` is, PLUS the `ownership: 'business_unit'` tier which
241+
* has an owning unit but deliberately no owning person (inventory, asset
242+
* ledgers, departmental budgets). Withheld for `org` / `none` exactly like
243+
* `owner_id`. Shaped like `organization_id`, not like `owner_id`
244+
* (`readonly: true`, `hidden: true`): it is a SERVER-STAMPED scope anchor,
245+
* and the stamping middleware (ADR-0117 D2/D4) has not landed — so the
246+
* column is provisioned but inert. See {@link applySystemFields}' injection
247+
* site for why that shape presumes nothing about the undecided D2 policy.
236248
*/
237249
/**
238250
* Column definitions for the audit-provenance family, keyed by the spec's
@@ -299,6 +311,15 @@ const AUDIT_FIELD_GOVERNANCE: Record<AuditProvenanceField, Record<string, unknow
299311
AUDIT_PROVENANCE_FIELDS.map((name) => [name, { readonly: true, system: true }]),
300312
) as unknown as Record<AuditProvenanceField, Record<string, unknown>>;
301313

314+
/**
315+
* [ADR-0117 D1] The injected BU-ownership column name, spelled out so it greps
316+
* from this file, and annotated with the spec's registered protocol name so the
317+
* two cannot drift: a rename in `SystemFieldName.OWNING_BUSINESS_UNIT_ID` makes
318+
* this line a compile error rather than a silently orphaned injection.
319+
*/
320+
const OWNING_BUSINESS_UNIT_FIELD: typeof SystemFieldName.OWNING_BUSINESS_UNIT_ID =
321+
'owning_business_unit_id';
322+
302323
export function applySystemFields(
303324
schema: ServiceObject,
304325
opts: { multiTenant: boolean }
@@ -355,12 +376,42 @@ export function applySystemFields(
355376
// authors silently ship objects with no working ownership at all.
356377
// `ownership` is now a declared ObjectSchema field (record-ownership model),
357378
// so it reads off the typed schema — no `as any` (#3175).
358-
const ownership = schema.ownership;
359-
const wantOwner =
360-
ownership !== 'org' &&
361-
ownership !== 'none' &&
362-
!(schema as any).managedBy &&
363-
!schema.name.startsWith('sys_');
379+
//
380+
// [ADR-0117 D1 / #5677] Widened to `string` on purpose. The spec enum is
381+
// `'user' | 'org' | 'none'` TODAY; D1's fourth tier `'business_unit'` is
382+
// declared in #5678, strictly AFTER this PR — that ordering is the whole
383+
// point of #5677: the engine must recognise the tier BEFORE the schema can
384+
// emit it, or the tier's first appearance would be judged by the branch
385+
// below and get the INVERSE of what D1 declares. Without the widening `tsc`
386+
// rejects the comparison as a no-overlap literal test; with it the engine is
387+
// ready and the enum lands into a runtime that already honours it.
388+
const ownership: string | undefined = schema.ownership;
389+
390+
// Platform-managed tables and the `sys_*` namespace never carry a per-record
391+
// ownership anchor, whichever tier is declared — unchanged, and shared by
392+
// both anchors below so they cannot drift apart.
393+
const ownershipEligible = !(schema as any).managedBy && !schema.name.startsWith('sys_');
394+
395+
// [ADR-0117 D1 / #5677] POSITIVE LIST, deliberately — this used to read
396+
// `ownership !== 'org' && ownership !== 'none'`, i.e. a DENY-list, so ANY
397+
// value outside the two exclusions fell through to "inject `owner_id`".
398+
// That default is safe only while the enum has exactly three members: D1's
399+
// `business_unit` tier means "owned by a UNIT, not a person" (`owner_id` ❌,
400+
// `owning_business_unit_id` ✅), and under the deny-list it would have been
401+
// stamped with `owner_id` — the exact inverse. Behaviour for the three
402+
// values that exist today is IDENTICAL (`undefined`/`user` inject, `org`/
403+
// `none` do not); the change is only that a NEW tier no longer inherits the
404+
// owner branch by accident.
405+
const wantOwner = ownershipEligible && (ownership === undefined || ownership === 'user');
406+
407+
// [ADR-0117 D1] The BU anchor covers the owner tiers PLUS `business_unit`:
408+
//
409+
// ownership owner_id owning_business_unit_id
410+
// undefined/user ✅ ✅
411+
// business_unit ❌ ✅
412+
// org ❌ ❌
413+
// none ❌ ❌
414+
const wantOwningBusinessUnit = wantOwner || (ownershipEligible && ownership === 'business_unit');
364415

365416
const additions: Record<string, any> = {};
366417
// Platform-owned field settings that must WIN over a declared field, rather
@@ -436,6 +487,52 @@ export function applySystemFields(
436487
};
437488
}
438489

490+
// [ADR-0117 D1] Record-level business-unit ownership. Shaped after
491+
// `organization_id` (server-stamped scope anchor), NOT after `owner_id`
492+
// (a user-assignable business field) — the two differ in exactly the keys
493+
// that decide who may write and whether it renders:
494+
//
495+
// • `readonly: true` — D3 requires the value to be derived and validated
496+
// server-side (`record.organization_id` must equal the unit's org), so a
497+
// client-supplied value is overwritten, never trusted. Reassignment is a
498+
// TRANSFER-class operation gated on `allowTransfer` (D4), which has not
499+
// landed; until it does, no client write path may set this column.
500+
// • `hidden: true` — nothing stamps the column yet (D2's policy is
501+
// undecided, D8's backfill unwritten), so it is provisioned-but-inert.
502+
// Surfacing a permanently-NULL lookup on every business object's layout
503+
// would advertise a capability the runtime does not deliver (Prime
504+
// Directive #10). Presentation is a one-key flip for the PR that lands
505+
// stamping — a capability grant is not.
506+
// • `required: false` — nullable, like every other injected column.
507+
//
508+
// ⚠️ None of the three presumes ADR-0117 D2 (`pinned`/`follow_owner`/
509+
// `transferable`), which is NOT YET RULED. They are the fail-closed shape:
510+
// they grant no capability, so every D2 outcome remains reachable. D2's
511+
// `owningBusinessUnit.required` is an INSERT-time rejection rule, not column
512+
// nullability — it cannot be read off this definition either. `system: true`
513+
// writes (the future stamping middleware, seeds) are checked downstream of
514+
// `readonly`, so the flag blocks clients without blocking the platform —
515+
// exactly how `organization_id` is stamped today.
516+
//
517+
// No index: the hierarchical predicate that would use one (`… IN (units)`,
518+
// D6) ships with the enterprise scope resolver. An index on a column nothing
519+
// writes and nothing filters is dead weight — the same reasoning that gates
520+
// `organization_id`'s index on `multiTenant`.
521+
if (wantOwningBusinessUnit && !schema.fields?.[OWNING_BUSINESS_UNIT_FIELD]) {
522+
additions[OWNING_BUSINESS_UNIT_FIELD] = {
523+
type: 'lookup',
524+
reference: 'sys_business_unit',
525+
label: 'Owning Business Unit',
526+
required: false,
527+
hidden: true,
528+
readonly: true,
529+
system: true,
530+
description:
531+
'Record-level business-unit ownership (ADR-0117 D1). Server-stamped scope anchor; ' +
532+
'NULL until the stamping middleware lands.',
533+
};
534+
}
535+
439536
if (Object.keys(additions).length === 0 && Object.keys(overrides).length === 0) return schema;
440537

441538
return {

0 commit comments

Comments
 (0)