feat(approvals): expose the pending node's lockRecord policy on the request row (#3814, objectui#2902)

.changeset/approvals-expose-lock-record.md

@@ -0,0 +1,37 @@
+---
+"@objectstack/spec": minor
+"@objectstack/plugin-approvals": minor
+---
+
+feat(approvals): expose the pending node's `lockRecord` policy on the request row (#3814, objectui#2902)
+
+An approval node declares `lockRecord` (default `true`), and the record-lock
+`beforeUpdate` hook enforces exactly that: `lockRecord: false` and the record
+stays writable for the whole time the node waits. The behavior was correct and
+has been since Phase B — but it was **invisible to every client**.
+
+`rowFromRequest` parses `node_config_json` and projects a whitelist out of it
+(`__flowLabel`, `__nodeLabel`, `__round`, `escalation.timeoutHours`,
+`decisionOutputs`). `lockRecord` was never in that list, and no other field on
+`ApprovalRequestRow` carried the lock either. So the strongest thing a console
+could learn from `GET /approvals/requests` was *"a pending request exists"* —
+from which it can only assume the record is locked.
+
+That assumption is wrong on every opted-out node, and a flow that chains nodes
+with different policies makes it visibly wrong: the same UI state renders for
+"you may edit this" and "the server will reject your save with `RECORD_LOCKED`".
+The console has no third option — guessing the other way would offer an edit
+that dies on save.
+
+`ApprovalRequestRow` now carries **`lock_record: boolean`**, read from the same
+snapshot the hook reads, with the same `!== false` default. Present on every
+service read (`openNodeRequest` / `getRequest` / `listRequests`), so the flag a
+client renders and the rule the server applies cannot drift.
+
+Additive and backward compatible — nothing to migrate. A client that wants
+node-accurate lock state reads `request.lock_record`; treat `undefined` (an
+older backend) as locked, which is the pre-existing behavior.
+
+The showcase's `showcase_budget_approval` now declares `lockRecord: false` on
+its single-approver Manager Review and keeps `true` on the multi-approver
+Executive Review, so both policies are exercised in one flow.

examples/app-showcase/src/automation/flows/index.ts

@@ -186,7 +186,15 @@ export const BudgetApprovalFlow = defineFlow({
  config: {
  approvers: [{ type: 'position', value: 'manager' }],
  behavior: 'first_response',
- lockRecord: true,
+ // Deliberately UNLOCKED, and the counterpart to `exec_review` below —
+ // together they dogfood both record-lock policies in one flow
+ // (objectui#2902). A single-approver step like this is the case the
+ // flag exists for: the manager is expected to correct the budget
+ // narrative in place rather than send the whole thing back. It also
+ // matches this node's own revise loop, which assumes the record is
+ // reworkable. The console must show "in approval · editable" here and
+ // keep inline editing live; on `exec_review` it must show the lock.
+ lockRecord: false,
  // ADR-0044: at most two send-backs; the third auto-rejects.
  maxRevisions: 2,
  },
@@ -212,6 +220,8 @@ export const BudgetApprovalFlow = defineFlow({
  config: {
  approvers: [{ type: 'position', value: 'exec' }],
  behavior: 'unanimous',
+ // Locked, unlike `manager_review` — a multi-approver sign-off must
+ // decide on a stable record, so edits are refused until it completes.
  lockRecord: true,
  },
  },

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

@@ -169,6 +169,40 @@ describe('ApprovalService (node era)', () => {
  expect(JSON.parse(raw.node_config_json)).toMatchObject({ behavior: 'first_response', lockRecord: true });
  });
 
+ // ── record-lock policy on the read row (objectui#2902) ──────────
+ //
+ // The lock is enforced in `lifecycle-hooks.ts` off `node_config_json`, but
+ // the row projection used to drop the flag entirely — so a client could see
+ // "a pending request exists" and nothing more, and had to assume every
+ // pending node locked the record. Chaining nodes with different policies
+ // made that visibly wrong. These pin the flag onto every read path.
+
+ it('lock_record: true when the node locks (the schema default)', async () => {
+ const req = await svc.openNodeRequest(openInput(['u9']), CTX);
+ expect(req.lock_record).toBe(true);
+ const [listed] = await svc.listRequests({ object: 'opportunity', recordId: 'opp1' }, SYS);
+ expect(listed.lock_record).toBe(true);
+ expect((await svc.getRequest(req.id, SYS))!.lock_record).toBe(true);
+ });
+
+ it('lock_record: false when the node opts out — the same read the hook honors', async () => {
+ const req = await svc.openNodeRequest(openInput(['u9'], {}, { lockRecord: false }), CTX);
+ expect(req.lock_record).toBe(false);
+ const [listed] = await svc.listRequests({ object: 'opportunity', recordId: 'opp1' }, SYS);
+ expect(listed.lock_record).toBe(false);
+ expect((await svc.getRequest(req.id, SYS))!.lock_record).toBe(false);
+ });
+
+ it('lock_record: an unset lockRecord reads as locked, matching the hook default', async () => {
+ // The hook allows the write only on an explicit `=== false`; the flag must
+ // default the same way or the UI would offer an edit the server rejects.
+ const req = await svc.openNodeRequest(
+ { ...openInput(['u9']), config: { approvers: [{ type: 'user' as const, value: 'u9' }], behavior: 'first_response' as const } } as any,
+ CTX,
+ );
+ expect(req.lock_record).toBe(true);
+ });
+
  it('openNodeRequest: deduplicates a pending request per (object, record)', async () => {
  await svc.openNodeRequest(openInput(['u9']), CTX);
  await expect(svc.openNodeRequest(openInput(['u9'], { runId: 'run_2' }), CTX))

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

@@ -263,6 +263,14 @@ function rowFromRequest(row: any): ApprovalRequestRow {
  sla_due_at: slaDueAt(row.created_at, cfg),
  // ADR-0044 revision round (rides the config snapshot; absent ⇒ round 1).
  round: typeof cfg?.__round === 'number' ? cfg.__round : undefined,
+ // objectui#2902: the node's record-lock policy. The lock is enforced
+ // server-side in `lifecycle-hooks.ts` off THIS SAME snapshot with the
+ // same `!== false` default, so the flag a client renders and the rule the
+ // server applies can never drift. Without it a console can only see
+ // "a pending request exists" and has to assume the record is locked —
+ // which mislabels every `lockRecord: false` node as locked and hides an
+ // edit the server would have accepted.
+ lock_record: cfg?.lockRecord !== false,
  // #3447 P2: the node's author-declared decision outputs, surfaced so a
  // decision UI can render input fields for them and POST `outputs` on
  // approve/reject. Per-request (each node declares its own), which is why

packages/spec/src/contracts/approval-service.ts

@@ -139,6 +139,24 @@ export interface ApprovalRequestRow {
  * `node_config_json` snapshot (`__round`), so no schema migration.
  */
  round?: number;
+ /**
+ * Whether THIS node's pending request locks the target record from edits
+ * (objectui#2902). Mirrors the `lockRecord` policy the record-lock
+ * `beforeUpdate` hook enforces, read from the same `node_config_json`
+ * snapshot the hook reads — so a client never has to guess, and never
+ * disagrees with the server.
+ *
+ * `lockRecord` defaults to `true` (see `ApprovalNodeConfigSchema`), so this
+ * is `false` only when the node explicitly opted out. Always present on a
+ * service read; a client that gets `undefined` is talking to a pre-#3814
+ * backend and should fail closed (assume locked) rather than offer an edit
+ * the server will reject with `RECORD_LOCKED`.
+ *
+ * Node-scoped, not request-scoped in spirit: a flow chaining several
+ * approval nodes with different policies produces one request per node, and
+ * each carries its own value.
+ */
+ lock_record?: boolean;
  /**
  * Server-computed decision aggregation progress (#3266, single-request reads
  * of PENDING requests only). Present when the node's behavior aggregates