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

An approval node declares `lockRecord` (default `true`) and the record-lock
`beforeUpdate` hook enforces exactly that — `lockRecord: false` and the record
stays writable while the node waits. Correct since Phase B, and invisible to
every client.

`rowFromRequest` parses `node_config_json` but 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. 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 is wrong on every opted-out node,
and a flow chaining nodes with different policies makes it visibly wrong: one
UI state for "you may edit this" and "your save dies with RECORD_LOCKED".

`ApprovalRequestRow` now carries `lock_record: boolean`, read from the same
snapshot the hook reads with the same `!== false` default, on every service
read. The flag a client renders and the rule the server applies cannot drift.

Additive and backward compatible. The showcase's `showcase_budget_approval`
declares `lockRecord: false` on its single-approver Manager Review and keeps
`true` on the multi-approver Executive Review, so one flow exercises both.

Closes #3814

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: baozhoutao

.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

@@ -183,6 +183,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

@@ -324,6 +324,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