Skip to content

Commit bb61df2

Browse files
committed
Merge origin/main into the 2i PR B branch (#26137)
Main re-recorded six registry packages under scripts/sdui-manifest.sources/ (#26274, #26358) while the manifest's bytes stayed the same: six modify/delete conflicts against this PR's deletion of those records, resolved by deleting them. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6SEitNngAMugGbuJHWsDS
2 parents 15f5e27 + 6390fe5 commit bb61df2

73 files changed

Lines changed: 1906 additions & 463 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@object-ui/types': minor
3+
'@object-ui/react': minor
4+
'@object-ui/sdui-parser': minor
5+
'@object-ui/app-shell': patch
6+
---
7+
8+
feat(types,react,sdui-parser,app-shell)!: the node-level `visibleOn` / `hiddenOn` / `disabledOn` spellings are retired; `visibleWhen` / `hidden` / `disabled` are the one predicate each
9+
10+
Clause-②: no (narrowing)
11+
Metadata-protocol: narrowed
12+
13+
<!-- adr-0087: not-required (no-migration-prescription) The three keys are `@object-ui/types` schema members on objectui's own node face, not an `@objectstack/spec` key, object definition, stored row or stored shape: the spec declares `visibleWhen` as the one conditional-visibility predicate at every layer (ADR-0089) and never declared a `*On` suffix on a page component, so the ADR-0087 registries in `packages/spec` have no entry to write and `objectstack migrate meta` has nothing to rewrite. Each old spelling is refused by name on both faces (the TypeScript compiler, the Zod mirror behind `objectui validate`), and the refusal names the surviving key; that is the same disposition the objectui retirements took when the package merged into this repository (maintainer ruling 28 = B on objectstack#25831). The guidance below is for the author of a page, not for a metadata upgrader. The other categories are closed on facts: the four packages publish (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and what narrows is a declared schema key on a published metadata face, not a runtime-only interface, a type annotation or a runtime export (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
14+
15+
`@objectstack/spec` declares exactly one conditional-visibility predicate at every layer, `visibleWhen`, and objectui carried three spellings of its own beside it: `visibleOn` beside `visibleWhen`, `hiddenOn` beside `hidden`, `disabledOn` beside `disabled`. Each pair was evaluated by the same code with the same polarity, so the second spelling added a dialect and nothing else. The three are retired, with no alias window.
16+
17+
- **`@object-ui/types`**: `BaseSchema.visibleOn`, `hiddenOn` and `disabledOn` are `never` on the TypeScript face and alias refusals on the Zod mirror. A document that writes one is refused at that key, and the message names the key that survives (`visibleWhen`, `hidden`, `disabled`), says the polarity is the same, and says the value is unchanged. `objectui validate` reads the same mirror, so it prints the same refusal. The `object-view` table slot's refusal of the same three keys now names the surviving node key too (write `visibleWhen` / `hidden` / `disabled` on the object-view node itself) instead of sending the author to a retired spelling. Boolean `visible` stays: the spec declares it on actions, navigation entries, tabs, alerts and settings, and the renderer keeps evaluating it.
18+
- **`@object-ui/react`**: `SchemaRenderer`'s visibility chain has three legs, `visibleWhen` and `visible` (shown when truthy) and `hidden` (hidden when truthy), and the enablement gate has one, `disabled`. A stale retired key that reaches the renderer unvalidated is not consulted and is held out of the component's props, so it neither gates the node nor lands in the DOM.
19+
- **`@object-ui/sdui-parser`**: the three keys are no longer base props. `validateTree` reports each as an `unknown-prop` diagnostic that names the surviving key, so a TSX tree is told the same thing a JSON document is. `checkRetiredVisibilityDialect` and `RETIRED_VISIBILITY_DIALECTS` are exported for that reading.
20+
- **`@object-ui/app-shell`**: the command palette reads a navigation entry's `visible` only; the `visibleOn` fallback it carried read a key `NavigationItem` never declared.
21+
22+
**BREAKING** (an accept-set narrowing) for any page, view or component document that writes `visibleOn`, `hiddenOn` or `disabledOn` on a node. No shipped example, fixture or skill example in this repository wrote one; the ones in the documentation and the `objectui` skill are rewritten in this change.
23+
24+
- **Before.** The three keys parsed, were evaluated beside their canonical twin, and the docs taught both spellings.
25+
- **Now.** Each is refused by name on `objectui validate`, on the Zod mirror, by the TypeScript compiler and by the SDUI parser, and the renderer does not read it.
26+
- **Fix.** Rename the key and keep the value: `visibleOn` becomes `visibleWhen`, `hiddenOn` becomes `hidden`, `disabledOn` becomes `disabled`. A form field's `fields[].visibleOn` is a different, view-level slot and is unchanged.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/plugin-grid': patch
3+
---
4+
5+
fix(plugin-grid): a bulk action's confirm sentence formats its numbers and dates in the viewer's display locale
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A bulk `confirmText` such as `Set the amount to {{ amount | currency:EUR }} on {{ count }} records?` formatted its numbers and dates in `en-US` for every viewer, while the same action's single-record confirm formats them in the viewer's language. `BulkActionDialog` now reads `useDisplayLocale()`, the locale every date, number and currency renderer and the console's action runners read, and `bulkConfirmText` takes it as an optional third argument, handed to `fillTemplateHoles`'s locale channel apart from the template roots. A German viewer reads `1.234.567,50 €`; with no locale handed, the sentence formats in `en-US` as before.
11+
12+
What to do: nothing.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/plugin-approvals': patch
4+
'@objectstack/rest': patch
5+
'@objectstack/client': patch
6+
---
7+
8+
feat(plugin-approvals,rest,client): a record-anchored approval lock read, `{ locked }` for every caller who can read the record
9+
10+
Clause-②: yes (widening)
11+
Metadata-protocol: none
12+
13+
A member who could read and edit a record but was not on its approval was offered an edit the server then refused with `409 RECORD_LOCKED`. The lock's only carrier was the request row's `lock_record`, and that row is rightly not served to a non-participant unless the object turns on `approvalsVisibleToReaders`. The lock now has a carrier of its own.
14+
15+
- **`@objectstack/spec`.** `IApprovalService` gains the optional member `getRecordLock(object, recordId, context)`, which answers `{ locked: boolean }`, or `null` when the caller cannot read the record. No exported schema or type is added.
16+
- **`@objectstack/plugin-approvals`.** `ApprovalService.getRecordLock` reads the record as the caller. A record the caller cannot read answers `null`, the same answer as an id that does not exist. A caller with no read on the object is refused for every id, whether it exists or not. `locked` is `true` while a pending request's node has `lockRecord` other than `false`. The lock judgement is now one function, `nodeConfigLocksRecord` (not exported from the package), and the record-lock hook, the request row's `lock_record` and the new read all call it. Request content stays behind `approvalsVisibleToReaders`: the answer carries no request id, node, approvers or `pending` flag.
17+
- **`@objectstack/rest`.** New route `GET /api/v1/approvals/records/:object/:recordId/lock`. It answers `{ success: true, data: { locked } }`. A `null` answers `404 RECORD_NOT_FOUND` with one body, whether the record is unreadable or missing. A service without the member answers `501`. The route's closed query-parameter set is empty, so any query parameter is refused with a `400`. The route is ledgered as `sdk`.
18+
- **`@objectstack/client`.** `client.approvals.getRecordLock(object, recordId)` calls that route.

‎content/docs/automation/approvals.mdx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,27 @@ never deadlock against itself. The exemption is keyed on run identity rather tha
376376
elevation, so a `runAs:'user'` run stays row-level-security scoped while it
377377
writes — it does not become a system write.
378378

379+
**Reading the lock.** Anyone who can read the record can ask whether it is
380+
locked, including a member who is not on the approval and so is served no
381+
request row:
382+
383+
```bash
384+
curl -b cookies.txt \
385+
"https://your-app.example.com/api/v1/approvals/records/expense/rec_123/lock"
386+
# → { "success": true, "data": { "locked": true } }
387+
```
388+
389+
The answer is `locked` and nothing else: no request, approvers or history,
390+
which stay behind the object's `approvalsVisibleToReaders` switch. `locked` is
391+
`true` while a pending request's node has `lockRecord` other than `false`. That
392+
is the same check the server applies when it refuses an edit with `409
393+
RECORD_LOCKED`, so a page that reads it never offers an edit the server will
394+
refuse. The record is read as the caller: a record they cannot read answers
395+
`404 RECORD_NOT_FOUND`, the same body a record id that does not exist gets. The
396+
SDK call is `client.approvals.getRecordLock(object, recordId)`, and the service
397+
member is `IApprovalService.getRecordLock`. An approvals service without that
398+
member answers `501`.
399+
379400
Only `approvers` is required on the node; everything else has a default
380401
(`behavior: 'first_response'`, `lockRecord: true`, `maxRevisions: 3`).
381402

‎packages/client/src/client.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,15 @@ describe('Approvals namespace (ADR-0019)', () => {
704704
expect(url).toContain('/api/v1/approvals/requests/req-1');
705705
});
706706

707+
it('[#26314] reads a record\'s approval lock off the envelope, path segments encoded', async () => {
708+
const { client, fetchMock } = createMockClient({ success: true, data: { locked: true } });
709+
const result = await client.approvals.getRecordLock('order', 'rec/1');
710+
expect(result).toEqual({ locked: true });
711+
const [url, opts] = fetchMock.mock.calls[0];
712+
expect(String(url)).toBe('http://localhost:3000/api/v1/approvals/records/order/rec%2F1/lock');
713+
expect(opts?.method ?? 'GET').toBe('GET');
714+
});
715+
707716
it('should record an approve decision', async () => {
708717
const { client, fetchMock } = createMockClient({
709718
request: { id: 'req-1', status: 'approved' }, finalized: true, decision: 'approve', resumed: true

‎packages/client/src/index.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6260,6 +6260,21 @@ export class ObjectStackClient {
62606260
return this.unwrapResponse<ApprovalRequestRow>(res);
62616261
},
62626262

6263+
/**
6264+
* Whether a record is held by a locking approval — `{ locked }` and
6265+
* nothing else, answered to every caller who can read the record, whether
6266+
* or not they take part in the approval (`IApprovalService.getRecordLock`).
6267+
* A record the caller cannot read answers 404, exactly as a record id that
6268+
* does not exist; a deployment without the read answers 501.
6269+
*/
6270+
getRecordLock: async (object: string, recordId: string): Promise<{ locked: boolean }> => {
6271+
const route = this.getRoute('approvals');
6272+
const res = await this.fetch(
6273+
`${this.baseUrl}${route}/records/${encodeURIComponent(object)}/${encodeURIComponent(recordId)}/lock`,
6274+
);
6275+
return this.unwrapResponse<{ locked: boolean }>(res);
6276+
},
6277+
62636278
/**
62646279
* Record an approve decision on a request. Finalises the request when the
62656280
* node's behaviour is satisfied and resumes the owning flow run.

‎packages/plugins/plugin-approvals/src/approval-service.ts‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ import {
120120
type FieldVisibilitySource,
121121
} from './payload-redaction.js';
122122
import { APPROVAL_REQUEST_CHILD_OBJECTS, type RequestVisibilitySource } from './request-read-gate.js';
123+
import { nodeConfigLocksRecord, recordHeldByLockingApproval } from './lifecycle-hooks.js';
123124
// The ADR-0043 pages — the ONE copy of their text, rendered by the plugin's
124125
// self-hosted mount and by `handleActionPage` alike.
125126
import { renderConfirmPage, renderResultPage } from './action-link-pages.js';
@@ -1280,13 +1281,13 @@ function rowFromRequest(row: any): ApprovalRequestRow {
12801281
// ADR-0044 revision round (rides the config snapshot; absent ⇒ round 1).
12811282
round: typeof cfg?.__round === 'number' ? cfg.__round : undefined,
12821283
// objectui#2902: the node's record-lock policy. The lock is enforced
1283-
// server-side in `lifecycle-hooks.ts` off THIS SAME snapshot with the
1284-
// same `!== false` default, so the flag a client renders and the rule the
1285-
// server applies can never drift. Without it a console can only see
1286-
// "a pending request exists" and has to assume the record is locked —
1287-
// which mislabels every `lockRecord: false` node as locked and hides an
1288-
// edit the server would have accepted.
1289-
lock_record: cfg?.lockRecord !== false,
1284+
// server-side in `lifecycle-hooks.ts` off THIS SAME snapshot by the SAME
1285+
// judgement (`nodeConfigLocksRecord`, #26314), so the flag a client renders
1286+
// and the rule the server applies can never drift. Without it a console can
1287+
// only see "a pending request exists" and has to assume the record is
1288+
// locked — which mislabels every `lockRecord: false` node as locked and
1289+
// hides an edit the server would have accepted.
1290+
lock_record: nodeConfigLocksRecord(cfg),
12901291
// #3447 P2: the node's author-declared decision outputs, surfaced so a
12911292
// decision UI can render input fields for them and POST `outputs` on
12921293
// approve/reject. Per-request (each node declares its own), which is why
@@ -8641,6 +8642,46 @@ export class ApprovalService implements IApprovalService {
86418642
return this.loadRequest(requestId, context, true);
86428643
}
86438644

8645+
/**
8646+
* [#26314] `IApprovalService.getRecordLock` — is this record held by a
8647+
* locking approval, for EVERY caller who can read the record (#25934 ruling
8648+
* A). A non-participant is told the record is locked before they edit it,
8649+
* instead of being offered an edit the lock hook then refuses.
8650+
*
8651+
* ## Who is answered — the #8652 anchor, read as the caller
8652+
*
8653+
* `engine.find(object, { where: { id }, context })` with the CALLER's
8654+
* context, exactly as {@link addRecordReaderVisibleIds} reads it: object CRUD
8655+
* read, then RLS. A row the caller may not see comes back empty and answers
8656+
* `null` — the very answer a record id that does not exist gets, because it
8657+
* is the same branch. ⛔ No path tells the two apart. A refusal of the read
8658+
* itself (no read on the object) is thrown, and is the same refusal for any
8659+
* id, existing or not. ⚠️ Probing with {@link SYSTEM_CTX} here would admit
8660+
* every authenticated user in the tenant.
8661+
*
8662+
* ## What is answered — the lock, and nothing about the request
8663+
*
8664+
* `{ locked }` only: no request id, node, participants or `pending`. Request
8665+
* content stays behind `approvalsVisibleToReaders` (#8652 ruling 5299823744,
8666+
* untouched). The judgement is the record-lock hook's own
8667+
* ({@link recordHeldByLockingApproval}), never a second copy of it.
8668+
*/
8669+
async getRecordLock(
8670+
object: string,
8671+
recordId: string,
8672+
context: ExecutionContext,
8673+
): Promise<{ locked: boolean } | null> {
8674+
const objectName = String(object ?? '').trim();
8675+
const id = String(recordId ?? '').trim();
8676+
if (!objectName || !id) return null;
8677+
// The anchor. As the CALLER — see the doc block.
8678+
const readable = await this.engine.find(objectName, {
8679+
where: { id }, fields: ['id'], limit: 1, context,
8680+
});
8681+
if (!Array.isArray(readable) || readable.length === 0) return null;
8682+
return { locked: await recordHeldByLockingApproval(this.engine, objectName, id) };
8683+
}
8684+
86448685
private async loadRequest(
86458686
requestId: string,
86468687
context: ExecutionContext,

‎packages/plugins/plugin-approvals/src/lifecycle-hooks.ts‎

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ function asIdList(id: unknown): Array<string | number> | null {
315315

316316
/** The pending request gating a record, plus its snapshotted node config. */
317317
async function pendingRequestFor(
318-
engine: MinimalEngine,
318+
engine: Pick<MinimalEngine, 'find'>,
319319
objectName: string,
320320
recordId: string,
321321
): Promise<any | null> {
@@ -333,7 +333,7 @@ async function pendingRequestFor(
333333

334334
/** Pending requests gating any of the records a write names by id. */
335335
async function pendingRequestsForRecords(
336-
engine: MinimalEngine,
336+
engine: Pick<MinimalEngine, 'find'>,
337337
objectName: string,
338338
recordIds: ReadonlyArray<string | number>,
339339
): Promise<any[]> {
@@ -388,6 +388,57 @@ async function pendingRequestsForObject(
388388
}
389389
}
390390

391+
/**
392+
* THE lock judgement: does a pending request whose node snapshotted `config`
393+
* lock its target record against edits?
394+
*
395+
* `lockRecord` defaults to `true` (`ApprovalNodeConfigSchema`), so a node locks
396+
* unless it explicitly opted out. This is the one place that reading lives
397+
* (#26314): the record-lock hook below enforces it, the request row mirrors it
398+
* as `lock_record` (`rowFromRequest` in `approval-service.ts`), and the
399+
* record-anchored read ({@link recordHeldByLockingApproval}) answers it — so
400+
* what a client is told and what the server refuses can never drift. ⛔ Never
401+
* re-spell `lockRecord !== false` at a call site.
402+
*/
403+
export function nodeConfigLocksRecord(config: unknown): boolean {
404+
return (config as { lockRecord?: unknown } | null | undefined)?.lockRecord !== false;
405+
}
406+
407+
/**
408+
* The approvals plugin's own bookkeeping objects are never locked: the hook
409+
* skips them with this cheap prefix test (it is a `beforeUpdate` guard, and
410+
* the delete linkage below excludes them at registration instead).
411+
*/
412+
function isApprovalBookkeepingObject(objectName: string): boolean {
413+
return String(objectName).startsWith('sys_approval');
414+
}
415+
416+
/**
417+
* [#26314] Is `recordId` of `objectName` held by a LOCKING pending approval —
418+
* would an ordinary edit of it be refused `RECORD_LOCKED`?
419+
*
420+
* Asked exactly as the record-lock hook asks it of a by-id write: the same
421+
* pending-request lookup ({@link pendingRequestsForRecords}, read as SYSTEM, so
422+
* the bookkeeping is never narrowed by a caller's visibility) and the same
423+
* judgement ({@link nodeConfigLocksRecord}). What it leaves out is what belongs
424+
* to a WRITE, not to the record: the opening run's own writes (#3456) and a
425+
* write touching only the status mirror are exempt there because of who writes
426+
* and what, and neither is a property of the record.
427+
*
428+
* ⚠️ It answers for the record, not for a caller, and reads nothing as one.
429+
* Whether the asker may learn the answer — may read the record at all — is the
430+
* caller's question to settle first (`ApprovalService.getRecordLock`).
431+
*/
432+
export async function recordHeldByLockingApproval(
433+
engine: Pick<MinimalEngine, 'find'>,
434+
objectName: string,
435+
recordId: string,
436+
): Promise<boolean> {
437+
if (isApprovalBookkeepingObject(objectName)) return false;
438+
const gating = await pendingRequestsForRecords(engine, objectName, [recordId]);
439+
return gating.some((pending) => nodeConfigLocksRecord(parseJson<unknown>(pending?.node_config_json, {})));
440+
}
441+
391442
/**
392443
* Narrow `candidates` (pending requests) to the ones whose record the caller's
393444
* predicate actually matches — asked of the engine as an intersection, so the
@@ -487,7 +538,7 @@ export function bindApprovalLockHook(
487538
engine.registerHook('beforeUpdate', async (ctx: any) => {
488539
const object = (ctx?.object ?? ctx?.objectName) as string | undefined;
489540
// No object name (shouldn't happen) or our own bookkeeping objects → skip.
490-
if (!object || String(object).startsWith('sys_approval')) return;
541+
if (!object || isApprovalBookkeepingObject(object)) return;
491542

492543
const data = (ctx?.input?.data ?? {}) as Record<string, unknown>;
493544
const changedFields = Object.keys(data).filter((k) => k !== 'id' && k !== 'updated_at');
@@ -538,7 +589,7 @@ export function bindApprovalLockHook(
538589
if (writerRun && pending?.flow_run_id && String(writerRun) === String(pending.flow_run_id)) continue;
539590

540591
const config = parseJson<any>(pending?.node_config_json, {});
541-
if (config?.lockRecord === false) continue;
592+
if (!nodeConfigLocksRecord(config)) continue;
542593

543594
// Allow when every changed field is the approval status mirror.
544595
const mirror = config?.approvalStatusField;

0 commit comments

Comments
 (0)