Skip to content

Commit b32ee3e

Browse files
feat(spec)!: approver and decision-output positions are names from the metadata registry (ADR-0131 D4) (#22710)
Part of #22682 Clause-②: yes Items 1 and 2 of the card: the approver `position` binding and the decision-output `position` become names, listed from the metadata registry (ADR-0131 D4). Item 3, the position form's `permissionSets` widget (`packages/spec/src/identity/position.form.ts`), waits for stage 1 of #15204, so #22682 remains open for it. Claim `6098135473`, dispatched by the `domain:spec` seat 2. ## What changes All in `packages/spec/src/automation/approval.zod.ts`. 1. **`ApproverValueBinding` gains a `registry` member**, `{ source: 'registry'; type: string }`: the value is the machine name of an item of metadata `type`. A picker lists `GET /api/v1/meta/TYPE`, which serves the code-declared and the environment-authored items alike (ADR-0131 D2), and commits the item's `name`. 2. **`APPROVER_VALUE_BINDINGS.position`** is `{ source: 'registry', type: 'position' }`. It was `{ source: 'record', object: 'sys_position', valueField: 'name' }`. The `satisfies` exhaustiveness check, under which every `ApproverType` member needs a binding, is kept. 3. **`APPROVER_VALUE_SOURCES`** projects a `registry` binding onto the wire as its own source, so the approval node schema's `approvers[].value` `xRef.sources.position` reads `{ source: 'registry', type: 'position' }`. Its type gains `{ source: 'registry'; type: string }`. `user`, `team` and `department` still project `data`. The docblock that said the registry "does not hold … `sys_position` ROWS" now says what each source is for: `data` for a directory row, `registry` for a catalog item. 4. **`DecisionOutputDef`'s `position` output collects position machine names.** The docblock said every picker's values were record ids. It now says each picker kind is sourced and committed exactly like the approver type of the same name, and the `type` and `multiple` `.describe()` strings say what each picker commits. The `resolveAs` and node-level `decisionOutputs` docblocks say the same. The generated reference `content/docs/references/automation/approval.mdx` is regenerated (`gen:docs`, the one artifact `check:generated` named). Departures from the suggested route, both deliberate: - **No `valueField` on the `registry` member.** A catalog item has no row id; its name is the only reference it has (ADR-0131 D4), so a `valueField` could only ever hold `name`, a key with nothing to choose. - **`type` is `string`, not `MetadataType`.** Importing the kernel's type, even type-only, pulled `kernel/metadata-plugin.zod.ts` and about 25 transitive modules into the generated skill reference index (`skills/objectstack-automation/references/_index.md`, a governed `skills/**` path). The import is dropped, and `approval.test.ts` holds every registry binding's `type` to `MetadataTypeSchema` instead. The runtime does not change. `plugin-approvals` routes a position approver, an escalation target and a `resolveAs: 'position'` value by filtering `sys_user_position.position` by name, and none of it reads the binding. ## Premises, re-verified on `origin/main` `243dd3c625` - `approval.zod.ts:206` was `position: { source: 'record', object: 'sys_position', valueField: 'name' }`. `APPROVER_VALUE_SOURCES` (`:238`) projected `record` to `{ source: 'data', object, valueField }`. `DecisionOutputDef` (`:576`) read "Picker values are record ids". `approval.test.ts:152`, `:193` and `:210` pinned those shapes. - **No registry binding kind existed.** The union was `record | enum | auto | trigger-field | expression | unsupported`. `git grep` for `APPROVER_VALUE_BINDINGS`, `APPROVER_VALUE_SOURCES` and `ApproverValueBinding` outside `packages/spec` finds one code reader, `packages/lint/src/validate-approval-approvers.ts:451`, which reads only `source === 'unsupported'` and is unaffected. The only registry-picker spelling elsewhere in the spec is the form-widget `ref:TYPE` vocabulary (`ref:object`, `ref:dataset`, `ref:component`), which is a widget key in `*.form.ts` and not a value binding. It is item 3's layer, so no second vocabulary is introduced here. - **Item 2's values are already names at run time.** `decideNode` hands outputs to the flow verbatim (`approval-service.ts` `acceptedOutputs`). The one reader that interprets a position value is an `expression` approver with `resolveAs: 'position'`, and that calls `expandPositionUsers(key)`, which filters `sys_user_position` by `position: NAME` (`approval-service.ts:2686`). A `sys_position` id would route the request to nobody. So item 2 is a contract and docblock correction with no value-shape change, and `plugin-approvals` is untouched. ## What objectui at the pinned `.objectui-sha` does with this Read at objectui `20c6d351ad74` (the pin) with `git show`. - **objectui bundles `@objectstack/spec@17.7.0` from npm.** Its lockfile resolves that version, and `packages/app-shell` and `packages/fields` declare `^17.6.0`. This change ships on the v18 prerelease line, which `^17` never resolves. So every reader that imports the constant still sees the old binding: `approverIdentity.ts` (`APPROVER_DIRECTORY_BINDINGS`, read by `useApproverDirectory`), `FlowReferenceField.tsx` (`KIND_TO_RECORD_LOOKUP`) and `decisionOutputParams.ts` (`OUTPUT_LOOKUP_OBJECTS`, its own hand-written table). - **The one reader that takes the binding off the wire** is the flow designer, which reads `xRef.sources` from the approval node schema the server publishes. `sourceOf` in `json-schema-to-fields.ts` accepts only `data`, `enum`, `auto`, `trigger-field`, `expression` and `unsupported`. It drops any other entry by design ("A dropped entry simply leaves the consumer on its local fallback"). `sources.position` is therefore absent, and `recordLookupFor` falls back to the bundled `KIND_TO_RECORD_LOOKUP.position`, a `sys_position` data lookup that commits `name`. That is today's control exactly: no throw and no free-text box, for as long as `sys_position` rows exist. - **If objectui moves to a spec carrying this change without the C9 follow-up**, `position` drops out of both spec-derived tables, because both keep only `source === 'data'` and skip anything else without throwing. The designer's position cell becomes a combobox with no suggestions, which is the free-text degradation the spec's comment records. `formatIdentity` stops prettifying `position:sales_manager` and middle-truncates it to `positi…ager`, and `useApproverDirectory` stops resolving labels and holders for position seats. Nothing throws. - **Ordering this implies:** this PR does not need to wait for objectui. The objectui follow-up (objectui#7611 C9) has to land in or before the objectui PR that moves objectui's spec dependency to the v18 line, and before #15204's stage 8 retires `sys_position`, because the pin's fallback lookup reads that object. The seat decides. - Today's objectui decision dialog builds the position output as a `lookup` on `sys_position` with no `idField`, so it commits the row id. Under the corrected contract that id routes a downstream `resolveAs: 'position'` approver to nobody. That reader is in the C9 follow-up's census (`decisionOutputParams`). This is a code reading, not a reproduction. ## Semver and the migration ledger - `Clause-②: yes`. The published `ApproverValueBinding` and the `APPROVER_VALUE_SOURCES` type each gain a member, and the `xRef.sources` payload gains the key `type`. - **BREAKING**, named in the changeset: the published `APPROVER_VALUE_SOURCES.position` entry changes shape for an existing reader. A renderer that keys on `source === 'data'`, as objectui's derived tables do, loses `position` until it reads `registry`. The changeset is `minor` on `@objectstack/spec`. Pre mode is on (tag `next`), and breaking changes ship as `minor` under the launch-window convention, as the stock does. - ADR-0087 disposition: `not-required (no-migration-prescription)`. No authorable key, export or stored value moves. A position approver's value and a position decision output were names at run time and stay names, so `objectstack migrate meta` has nothing to rewrite. - `api-surface/` and `export-origins/` are unchanged, because no export is added, removed or renamed. `check:api-surface` and `check:export-origins` pass. ## Tests The final head is `d4ca8f4762`. Every run below was taken on that head, after `origin/main` `9646991283` was merged in. Builds and tests ran through `scripts/pm/os-verify-lock.sh`, and each reading is the verdict line it printed. - `pnpm --filter @objectstack/spec build`, then `typecheck` (`tsc --noEmit`, the scripts typecheck and the test-layer typecheck): exit 0. - `pnpm --filter @objectstack/spec exec vitest run --project local --maxWorkers=2`, the whole local suite: **642 test files and 19,161 tests passed, 1 todo.** - `src/automation/approval.test.ts`: 66 of 66 passed. Changed pins: position binds `{ source: 'registry', type: 'position' }`, and every registry binding's `type` parses under `MetadataTypeSchema`. No binding names `sys_position`. `APPROVER_VALUE_SOURCES.position` is the registry source, and `registry` belongs to `position` alone. The published JSON schema's `xRef.sources.position` is the registry source. Every typed decision-output picker kind is an approver type bound to `record` or `registry`. - **Consumers, the downstream direction.** The `@objectstack/plugin-approvals` dependency closure was built with turbo (31 of 31 tasks). `@objectstack/lint`'s full suite passed: 135 files, 6,231 tests. Its reader of the bindings needs no change, and the existing "accepts the position approver type … silently" pin holds. The `plugin-approvals` read covered `approval-service.test.ts`, `approval-node.test.ts` and `approver-address-readers.test.ts`: 3 files and 378 tests passed. None of them was edited. - **Ablation**, run through `scripts/ablation-replace.mjs` in wrap mode on the committed head. The anchor `position: { source: 'registry', type: 'position' },` went from 1 to 0, and the record-binding replacement went from 0 to 1. The blob moved from `ba3680f2ddef` to `5195b44a4161`. `approval.test.ts` then went **red, 3 failed and 63 passed**, in the expected direction: the binding pin, the wire-source pin and the published-schema pin. The tool restored the file: its blob equals HEAD's `ba3680f2ddef`, and `git diff HEAD` is empty. A separate `git hash-object` check agrees. - **Gates.** `node scripts/pm/dispatch-gates.mjs` (no paths) derives 109 commands on this head. `--ran` with recorded exit codes reconciles **109 derived: 108 run, 1 NOT MEASURED, 0 unrun**. - Three gates needed a second run. `check:docs-transcript-drift` had refused for `PREREQUISITE NOT MET`, with lint not built yet. `check:query-options-erasure` and `check:type-check-debt` had hit a 300 s runner timeout. All three exit 0 on the second run. - `check:skill-examples` and `check:dts-closure` had read `client-react` and `verify` dists that the shared turbo cache replayed without declarations. Both exit 0 after a direct rebuild of those two packages, which this diff does not touch. - `check:generated` reports all 15 artifacts up to date. `check:adr-0087-registration` reports 1 declared-breaking changeset with a disposition. `check-changeset-no-major` exits 0. - **NOT MEASURED: `check:dual-build-cjs-loads`.** It refused with `PREREQUISITE NOT MET` (exit 3) because it needs every package built (`packages/plugins/organizations/dist` is absent). A repo-wide `pnpm build` is CI's run. This diff changes no build config, `exports` or package manifest. ## Acceptance notes - **The two sentences this change makes false are rewritten in this PR** (seat answer 6100715923, claim surface amended): `examples/app-showcase/src/automation/flows/approver-bindings.flow.ts:9-17` (position is a registry binding committed by machine name) and `content/docs/automation/approvals.mdx:336-339` (position decision-output picker values are machine names; `department` / `team` keep record ids). - `ApprovalEscalationSchema.escalateTo` carries `xRef: { kind: 'position' }` with no `sources`. objectui resolves its picker through the same spec-derived fallback, so it follows the binding with the C9 follow-up. No spec change is needed there. - `ApproverValueBinding`'s `record` member keeps `valueField: 'id' | 'name'`, although no record binding commits `name` now. Narrowing a published type for no reader would be churn. --- _Generated by [Claude Code](https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b6eb25f commit b32ee3e

6 files changed

Lines changed: 188 additions & 59 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec)!: an approver `position` and a decision-output `position` are catalog names, listed from the metadata registry (ADR-0131 D4)
6+
7+
Clause-②: yes
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) no authorable key, export or stored value changes: a position approver's value and a position decision output were names at run time and stay names; only the published picker binding changes shape -->
10+
11+
**BREAKING** for a renderer that derives its position picker from `APPROVER_VALUE_SOURCES`, shipped as `minor` under the repo's launch-window convention for breaking changes. Authored and stored metadata is untouched.
12+
13+
- **`APPROVER_VALUE_BINDINGS.position`** is a registry binding, `{ source: 'registry', type: 'position' }`. It was a record binding on `sys_position` rows that committed the row's `name`. `ApproverValueBinding` gains the `registry` member: the value is the machine name of an item of metadata `type`, and a catalog item has no other reference, so the member carries no `valueField`.
14+
- **`APPROVER_VALUE_SOURCES.position`**, the same binding on the wire (the `xRef.sources` of the approval node schema's `approvers[].value`), reads `{ source: 'registry', type: 'position' }`. It read `{ source: 'data', object: 'sys_position', valueField: 'name' }`, and its type gains the `{ source: 'registry'; type: string }` member. A picker for a `registry` source lists `GET /api/v1/meta/position`, which serves the code-declared and the environment-authored positions alike, and commits the item's `name`. `user`, `team` and `department` still read `data`.
15+
- **A `position` decision output collects position machine names**, the value the approver type of the same name takes. `DecisionOutputDef` said every picker collected record ids. The one reader that interprets a position output, an `expression` approver with `resolveAs: 'position'`, filters `sys_user_position.position` by name, so a `sys_position` id there routed the request to nobody. The `type` and `multiple` descriptions now say what each picker commits.
16+
- **The runtime does not change.** The approvals engine routes a position approver, an escalation target and a `resolveAs: 'position'` value by name, as it did.
17+
18+
Why: positions are declared metadata (ADR-0131 D2), and every reference to one names it and resolves through the registry (D4). The `sys_position` table is a mirror that the ADR-0131 cutover retires, so a picker that lists its rows lists the mirror, not the catalog.

‎content/docs/automation/approvals.mdx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,10 @@ decision, and `decision` / `requestId` are reserved.
333333

334334
A declaration can also be **typed** — `{ key: 'next_reviewers', type: 'user',
335335
multiple: true }` — which renders a multi-select user picker in the decision
336-
dialog instead of free text (`department` / `position` / `team` render the
337-
matching system-object picker; picker values are record ids, `multiple`
338-
collects an id array). The type shapes only the input widget; the whitelist
336+
dialog instead of free text (`department` / `team` render the matching
337+
system-object picker and commit record ids; `position` renders a picker over
338+
the position registry and commits position machine names; `multiple` collects
339+
an array of them). The type shapes only the input widget; the whitelist
339340
still works by `key`, so bare strings and typed declarations mix freely.
340341

341342
Add **`required: true`** when the next step cannot run without it — the very

‎content/docs/references/automation/approval.mdx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ const result = ApprovalDecision.parse(data);
105105
| :--- | :--- | :--- | :--- |
106106
| **key** | `string` | ✅ | Output key (the flow variable name under the node id) |
107107
| **label** | `string` | optional | Field label in the decision dialog |
108-
| **type** | `Enum<'text' \| 'user' \| 'department' \| 'position' \| 'team'>` | optional | Decision-dialog input widget (default 'text') |
109-
| **multiple** | `boolean` | optional | Collect multiple values (id array) |
108+
| **type** | `Enum<'text' \| 'user' \| 'department' \| 'position' \| 'team'>` | optional | Decision-dialog input widget (default 'text'). The user, department and team pickers commit record ids; the position picker commits position machine names |
109+
| **multiple** | `boolean` | optional | Collect multiple values (an array of what the picker commits) |
110110
| **required** | `boolean` | optional | Approver must supply this output to approve |
111111

112112
### Nested Shape: `ApprovalNodeConfig.escalation`
@@ -150,8 +150,8 @@ Approval step approver type. `manager` is resolved from the directory rather tha
150150
| :--- | :--- | :--- | :--- |
151151
| **key** | `string` | ✅ | Output key (the flow variable name under the node id) |
152152
| **label** | `string` | optional | Field label in the decision dialog |
153-
| **type** | `Enum<'text' \| 'user' \| 'department' \| 'position' \| 'team'>` | optional | Decision-dialog input widget (default 'text') |
154-
| **multiple** | `boolean` | optional | Collect multiple values (id array) |
153+
| **type** | `Enum<'text' \| 'user' \| 'department' \| 'position' \| 'team'>` | optional | Decision-dialog input widget (default 'text'). The user, department and team pickers commit record ids; the position picker commits position machine names |
154+
| **multiple** | `boolean` | optional | Collect multiple values (an array of what the picker commits) |
155155
| **required** | `boolean` | optional | Approver must supply this output to approve |
156156

157157

‎examples/app-showcase/src/automation/flows/approver-bindings.flow.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@ import { defineFlow } from '@objectstack/spec';
66
* #3508 dogfood: one node per RECORD-BACKED approver kind.
77
*
88
* The designer's Approvers → Value control used to query the metadata registry
9-
* (`GET /api/v1/meta/:type`) for every kind. `user` / `team` / `department` /
10-
* `position` are DATA rows in the platform's directory objects, not registry
11-
* entries, so candidates came back empty and the control degraded to a free-text
12-
* box — the author could only hand-type a userid. `APPROVER_VALUE_BINDINGS`
13-
* now publishes the data contract per kind (`xRef.sources`), and the designer
14-
* renders a real record lookup off it.
9+
* (`GET /api/v1/meta/:type`) for every kind. `user` / `team` / `department` are
10+
* DATA rows in the platform's directory objects, not registry entries, so
11+
* candidates came back empty and the control degraded to a free-text box — the
12+
* author could only hand-type a userid. `position` is a registry binding
13+
* (ADR-0131 D4): its candidates come from the position registry
14+
* (`GET /api/v1/meta/position`), and the value committed is the position's
15+
* machine name. `APPROVER_VALUE_BINDINGS` now publishes the data contract per
16+
* kind (`xRef.sources`) — a record lookup for the directory kinds, the registry
17+
* for `position` — for the designer to source each picker from.
1518
*
1619
* This flow is the authoring specimen for that surface: every kind whose control
1720
* changed gets a node, so opening it in the flow designer exercises each one.

‎packages/spec/src/automation/approval.test.ts‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
normalizeDecisionOutputs,
1919
} from './approval.zod';
2020
import { BUILTIN_MEMBERSHIP_ROLES } from '../identity/membership-role';
21+
import { MetadataTypeSchema } from '../kernel/metadata-plugin.zod';
2122

2223
describe('ApproverType', () => {
2324
it('should accept all valid approver types', () => {
@@ -147,9 +148,25 @@ describe('APPROVER_VALUE_BINDINGS — an approver value is picked from the recor
147148
// find('sys_business_unit', { id: value }) — a sys_business_unit id
148149
// (deliberately NOT a `sys_department`).
149150
expect(APPROVER_VALUE_BINDINGS.department).toEqual({ source: 'record', object: 'sys_business_unit', valueField: 'id' });
151+
});
152+
153+
it('binds position to the registry by machine name — a catalog item, not a sys_position row (ADR-0131 D4)', () => {
150154
// find('sys_user_position', { position: value }) — the position machine
151-
// NAME, not an id (portable across environments).
152-
expect(APPROVER_VALUE_BINDINGS.position).toEqual({ source: 'record', object: 'sys_position', valueField: 'name' });
155+
// NAME, the only reference a registry item has. No `valueField`: there is
156+
// no other column a catalog item could commit.
157+
expect(APPROVER_VALUE_BINDINGS.position).toEqual({ source: 'registry', type: 'position' });
158+
// Every registry binding names a type the registry actually serves — the
159+
// compile-time check the binding forgoes to keep approval.zod.ts free of
160+
// the kernel module graph.
161+
for (const [type, binding] of Object.entries(APPROVER_VALUE_BINDINGS)) {
162+
if (binding.source !== 'registry') continue;
163+
expect(MetadataTypeSchema.safeParse(binding.type).success, `'${type}' binds unknown metadata type '${binding.type}'`)
164+
.toBe(true);
165+
}
166+
// No binding names the retiring table as its source.
167+
for (const [type, binding] of Object.entries(APPROVER_VALUE_BINDINGS)) {
168+
expect((binding as { object?: string }).object, `'${type}' binds sys_position rows`).not.toBe('sys_position');
169+
}
153170
});
154171

155172
it('keeps the non-record kinds off the record-lookup path', () => {
@@ -189,8 +206,17 @@ describe('APPROVER_VALUE_SOURCES — where each picker finds its candidates, pub
189206
expect(APPROVER_VALUE_SOURCES.user).toEqual({ source: 'data', object: 'sys_user', valueField: 'id' });
190207
expect(APPROVER_VALUE_SOURCES.team).toEqual({ source: 'data', object: 'sys_team', valueField: 'id' });
191208
expect(APPROVER_VALUE_SOURCES.department).toEqual({ source: 'data', object: 'sys_business_unit', valueField: 'id' });
192-
// The one kind that commits something other than the primary key.
193-
expect(APPROVER_VALUE_SOURCES.position).toEqual({ source: 'data', object: 'sys_position', valueField: 'name' });
209+
});
210+
211+
it('routes position to the metadata registry, where positions are declared items (ADR-0131 D4)', () => {
212+
// `GET /api/v1/meta/position` serves both provenances; the picker commits
213+
// the item's name, which is what the engine filters sys_user_position by.
214+
expect(APPROVER_VALUE_SOURCES.position).toEqual({ source: 'registry', type: 'position' });
215+
// `registry` is position's alone: every directory kind stays on the data API.
216+
const registryKinds = Object.entries(APPROVER_VALUE_SOURCES)
217+
.filter(([, s]) => s.source === 'registry')
218+
.map(([type]) => type);
219+
expect(registryKinds).toEqual(['position']);
194220
});
195221

196222
it('keeps the non-record kinds off the lookup path and carries the closed enum inline', () => {
@@ -207,7 +233,26 @@ describe('APPROVER_VALUE_SOURCES — where each picker finds its candidates, pub
207233
const sources = schema?.properties?.approvers?.items?.properties?.value?.xRef?.sources;
208234
expect(sources).toBeDefined();
209235
expect(sources.department).toEqual({ source: 'data', object: 'sys_business_unit', valueField: 'id' });
210-
expect(sources.position).toEqual({ source: 'data', object: 'sys_position', valueField: 'name' });
236+
expect(sources.position).toEqual({ source: 'registry', type: 'position' });
237+
});
238+
});
239+
240+
// A typed decision output's picker is read downstream by an `expression`
241+
// approver's `resolveAs` of the same name, so it must source and commit what
242+
// that approver type takes. The docblock says so; this holds it: every picker
243+
// kind is an approver type whose binding is a lookup (record or registry).
244+
describe('DecisionOutputDef picker kinds follow the approver bindings of the same name', () => {
245+
const pickerKinds = (DecisionOutputDefSchema as any).shape.type.unwrap().options
246+
.filter((k: string) => k !== 'text') as string[];
247+
248+
it('names only approver types that are looked up, and covers the four it always had', () => {
249+
expect([...pickerKinds].sort()).toEqual(['department', 'position', 'team', 'user']);
250+
for (const kind of pickerKinds) {
251+
const binding = (APPROVER_VALUE_BINDINGS as Record<string, { source: string }>)[kind];
252+
expect(binding, `decision output '${kind}' has no approver binding`).toBeDefined();
253+
expect(['record', 'registry'], `decision output '${kind}' is bound to '${binding.source}'`)
254+
.toContain(binding.source);
255+
}
211256
});
212257
});
213258

0 commit comments

Comments
 (0)