fix(detail): distinguish "in approval (editable)" from locked, and stop losing write warnings (framework#3794)

The record page told the user the opposite of what the backend does, in both
directions.

The approval band rendered "Locked for approval" on any pending request,
ignoring the node's `lockRecord`. A `lockRecord: false` node is pending WITHOUT
locking — that is how an approver amends a record while deciding on it — so the
band hid the feature; meanwhile the header Edit button stayed enabled on a
genuinely locked record, letting the user fill a form the save would reject with
RECORD_LOCKED. `useRecordApprovals` now reads `locks_record` off the pending
request (framework#3794), `RecordDetailView` derives `approvalPending` and
`approvalLocked` separately and threads both through `InlineEditProvider`, and
the band picks the amber lock or a neutral "In approval (editable)" from the
host's signals. With no approvals-aware host the `approval_status` fallback is
unchanged and still assumes a lock.

The write-warning toast (framework #3431/#3455) never fired on
`batchTransaction`, which is the record form's save path for a master-detail
record — precisely where a user edits a `readonlyWhen`-locked field. It now
emits per event, resolving each back to its operation via the response `index`.
The toast is localized (en + zh) and worded by reason: a `readonly_when` strip
says the field is not editable in this record's current state, not that it is
read-only.

Finally, the user-state adapter no longer hand-stamps the server-managed
`updated_at`, which the server strips and reports — that fired a "Some fields
were not saved" warning about a field no user touched, on ordinary page loads.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: baozhoutao

.changeset/approval-band-truth-and-write-warnings.md

@@ -0,0 +1,51 @@
+---
+"@object-ui/plugin-detail": minor
+"@object-ui/app-shell": minor
+"@object-ui/data-objectstack": minor
+"@object-ui/react": minor
+"@object-ui/i18n": minor
+---
+
+fix(detail): say what the record actually allows — "in approval (editable)" vs locked, and warn on silently stripped fields (framework#3794)
+
+The Console reported record writability wrong in both directions during an
+approval, so a user had nothing to go on: what they *could* edit said "locked",
+and what they *couldn't* said "updated successfully".
+
+**"Locked for approval" was painted over every pending approval.** The band and
+the recall affordance rendered on any open request, ignoring the node's
+`lockRecord`. A node declaring `lockRecord: false` is pending *without* locking —
+the server's hook lets the write through on purpose, so the approver can amend
+the record as part of deciding on it — and the band told them not to bother. The
+mirror image was just as bad: on a genuinely locked record the header **Edit**
+button stayed live, so the user filled a whole form before the save came back
+`RECORD_LOCKED`.
+
+The pending request is now authoritative for both signals. `useRecordApprovals`
+reads `locks_record` (framework#3794, resolved from the same node-config snapshot
+the lock hook reads); `RecordDetailView` derives `approvalPending` and
+`approvalLocked` separately and threads both through `InlineEditProvider`
+(new `approvalPending` prop). The band renders the amber lock only when the
+record is really locked, a neutral "In approval (editable)" / 审批中（可编辑）
+otherwise, and the header Edit CTA is disabled on a locked record. With no
+approvals-aware host, the old `approval_status`-field fallback is unchanged and
+still assumes a lock — the safe direction.
+
+**Silently stripped fields now surface on the record form's save path.** The
+adapter emitted a write-warning for `create`/`update` responses carrying
+`droppedFields`, but not for `batchTransaction` — which is how the record form
+saves a master-detail record, i.e. the one surface where a user actually edits a
+`readonlyWhen`-locked field. `batchTransaction` now emits one warning per event,
+resolving each back to its operation via the response's `index`.
+
+The toast itself was hardcoded English and called every strip "read-only". It is
+now localized (`detail.writeStripped*`, en + zh) and worded by reason:
+`readonly_when` says the field is not editable *in this record's current state*,
+which is what actually happened — the field is editable in other states and the
+form rendered it as an ordinary input.
+
+**And it stopped crying wolf.** `createObjectStackUserStateAdapter` hand-stamped
+the server-managed `updated_at` on every recents/favorites write, which the
+server strips and reports — so the console popped "Some fields were not saved"
+about a field no user ever touched, on page loads, drowning the signal the toast
+exists for. It no longer sends the column; the server stamps it anyway.

packages/app-shell/src/hooks/useRecordApprovals.ts

@@ -32,6 +32,16 @@ export interface ApprovalRequestLite {
   pending_approvers?: string[] | null;
   submitted_at?: string;
   completed_at?: string | null;
+  /**
+   * Whether THIS request's node locks the record for writes (framework #3794).
+   * Read from the node-config snapshot's `lockRecord` by the same server that
+   * enforces the lock, so it cannot disagree with the `beforeUpdate` hook.
+   *
+   * `false` ⇒ pending but writable — the node deliberately lets the approver
+   * amend the record while deciding. Absent ⇒ an older server with no opinion;
+   * callers assume locked (the safe direction).
+   */
+  locks_record?: boolean;
 }
 
 interface UseRecordApprovalsResult {

packages/app-shell/src/providers/AdapterProvider.tsx

@@ -7,29 +7,59 @@
  * @module
  */
 
-import { useState, useEffect, type ReactNode } from 'react';
+import { useState, useEffect, useRef, type ReactNode } from 'react';
 import { toast } from 'sonner';
 import { ObjectStackAdapter, type WriteWarningEvent } from '@object-ui/data-objectstack';
 import { createAuthenticatedFetch } from '@object-ui/auth';
 import { AdapterCtx } from '@object-ui/react';
+import { useObjectTranslation } from '@object-ui/i18n';
 import { installSettleSignalGlobal, withSettleSignal } from '../observability/settleSignal';
 
 export { useAdapter } from '@object-ui/react';
 
+type TranslateFn = (key: string, options?: Record<string, unknown>) => string;
+
 /**
  * Turn a write-warning (framework #3431/#3455) into a human toast. The write
- * SUCCEEDED — some caller-supplied fields were legally stripped (read-only /
- * locked by state), so we tell the user rather than let it pass silently.
+ * SUCCEEDED — some caller-supplied fields were legally stripped, so we tell the
+ * user rather than let it pass silently.
+ *
+ * The REASON decides the wording (#3794). `readonly_when` is not "this field is
+ * read-only" — the field is editable in other states and the form rendered it
+ * as an ordinary input; what happened is that THIS record's current state locks
+ * it. Saying "read-only" there sends the user looking for a permission problem
+ * that doesn't exist. Fields are named by their machine name: the event carries
+ * no labels, and a wrong guess at a label is worse than an exact key.
  */
-function toastWriteWarning(ev: WriteWarningEvent): void {
-  const fields = Array.from(new Set(ev.droppedFields.flatMap((d) => d.fields)));
-  if (fields.length === 0) return;
-  const list = fields.join(', ');
-  const description =
-    fields.length === 1
-      ? `The read-only field “${list}” could not be changed and was not saved.`
-      : `${fields.length} read-only fields could not be changed and were not saved: ${list}.`;
-  toast.warning('Some fields were not saved', { description });
+function toastWriteWarning(ev: WriteWarningEvent, t: TranslateFn): void {
+  const byReason = new Map<string, string[]>();
+  for (const d of ev.droppedFields) {
+    const seen = byReason.get(d.reason) ?? [];
+    for (const f of d.fields) if (!seen.includes(f)) seen.push(f);
+    byReason.set(d.reason, seen);
+  }
+  const lines: string[] = [];
+  for (const [reason, fields] of byReason) {
+    if (fields.length === 0) continue;
+    const list = fields.join(', ');
+    lines.push(
+      reason === 'readonly_when'
+        ? t('detail.writeStrippedByState', {
+            fields: list,
+            defaultValue:
+              'Not editable in this record\'s current state, so it was not saved: {{fields}}',
+          })
+        : t('detail.writeStrippedReadonly', {
+            fields: list,
+            defaultValue: 'Read-only, so it was not saved: {{fields}}',
+          }),
+    );
+  }
+  if (lines.length === 0) return;
+  toast.warning(
+    t('detail.writeStrippedTitle', { defaultValue: 'Some fields were not saved' }),
+    { description: lines.join('\n') },
+  );
 }
 
 interface AdapterProviderProps {
@@ -44,6 +74,13 @@ interface AdapterProviderProps {
  */
 export function AdapterProvider({ children, adapter: externalAdapter }: AdapterProviderProps) {
   const [adapter, setAdapter] = useState<ObjectStackAdapter | null>(externalAdapter ?? null);
+  // The warning listener is registered ONCE (the adapter outlives a language
+  // switch), so read `t` through a ref instead of capturing it — otherwise a
+  // user who switches language mid-session keeps getting the old locale, and
+  // adding `t` to the effect deps would tear down and rebuild the adapter.
+  const { t } = useObjectTranslation();
+  const tRef = useRef(t);
+  tRef.current = t;
 
   useEffect(() => {
     if (externalAdapter) {
@@ -72,7 +109,9 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP
 
         // Surface silently-stripped write fields (#3431/#3455) as a toast so a
         // read-only value the user typed doesn't just vanish on save.
-        unsubscribeWriteWarning = a.onWriteWarning(toastWriteWarning);
+        unsubscribeWriteWarning = a.onWriteWarning((ev) =>
+          toastWriteWarning(ev, tRef.current as TranslateFn),
+        );
 
         await a.connect();
 

packages/app-shell/src/views/RecordDetailView.tsx

@@ -868,15 +868,31 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
     void approvalsRef.current.refresh();
   }, [recordInvalidationNonce]);
 
-  // The shared lock signal for the inline-edit session: the record's own
-  // `approval_status` (what the DetailView lock band keys off) OR an open
-  // pending request from the approvals API — the latter catches backends
-  // that lock via the request record without materializing the field.
-  const approvalLocked =
+  // "An approval is in flight" — the record's own `approval_status` mirror OR
+  // an open pending request from the approvals API (the latter catches backends
+  // that track approvals without materializing the field).
+  const approvalPending =
     (pageRecord as any)?.approval_status === 'pending' ||
     (pageRecord as any)?.approval_status === 'in_approval' ||
     !!approvals.pendingRequest;
 
+  // …and, separately, "the record is LOCKED for writes" (#3794). These are not
+  // the same statement: an approval node with `lockRecord: false` leaves the
+  // record editable on purpose — that is how an approver amends a record while
+  // deciding on it — and the server's lock hook honors it (`lockRecord === false`
+  // ⇒ the update goes through). Conflating them made the console claim "Locked
+  // for approval" on a record it would happily save, so approvers never tried.
+  //
+  // The pending REQUEST is authoritative when we have one: `locks_record` comes
+  // from the same node-config snapshot the server hook reads (framework #3794).
+  // With no request in hand — approvals plugin absent, still loading, or a
+  // field-only backend — fall back to the `approval_status` mirror and assume
+  // locked, which is the safe direction (worst case we hide an edit affordance
+  // instead of letting a user fill a form the server will reject).
+  const approvalLocked = approvals.pendingRequest
+    ? approvals.pendingRequest.locks_record !== false
+    : approvalPending;
+
   const approvalHandler = useCallback(async (action: ActionDef) => {
     const target = action.target || action.name;
     const params = (action.params && !Array.isArray(action.params))
@@ -1796,6 +1812,14 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
         // can't be stacked on top of the draft (two competing edit sessions
         // with no reconciliation).
         disableDuringInlineEdit: true,
+        // #3794 — an approval-LOCKED record refuses every write (the server
+        // answers RECORD_LOCKED), so opening the form only to be rejected on
+        // Save after filling a screen is wasted work. Disabled rather than
+        // hidden: the user should see the affordance exists and is off, with
+        // the lock band next to it saying why. Note this is the LOCK, not the
+        // mere presence of an approval — a `lockRecord: false` node keeps Edit
+        // live, which is the point of that setting.
+        disabled: approvalLocked,
         onClick: () => onEdit({ id: pureRecordId }),
       } as any);
     }
@@ -1919,10 +1943,15 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
             DetailView "Locked for approval" band renders from the SAME
             dual-source `approvalLocked` that gated `canEdit` — engaging even
             on backends that track the lock via approval requests only and
-            never materialize an `approval_status` field (objectui#2618). */}
+            never materialize an `approval_status` field (objectui#2618).
+            `approvalPending` is the separate "a request is in flight" signal
+            (#3794): on a `lockRecord: false` node it is true while `locked` is
+            false, and the band says "in approval (editable)" instead of lying
+            about a lock the server does not enforce. */}
         <InlineEditProvider
           canEdit={resolveRecordHeaderActionGates(objectDef, effectiveApiOperations).edit && !approvalLocked}
           locked={approvalLocked}
+          approvalPending={approvalPending}
           lockedReason={t('detail.lockedTooltip', {
             defaultValue: 'This record has a pending approval request; editing is locked',
           })}

packages/data-objectstack/src/index.ts

@@ -973,6 +973,41 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
     this.emitWriteWarning({ operation, resource, ...(id !== undefined ? { id } : {}), droppedFields });
   }
 
+  /**
+   * Same, for the cross-object transactional batch (framework #3794). Its
+   * response hangs the events off a top-level `droppedFields` list, each tagged
+   * with the `index` of the operation it came from — `results` entries are bare
+   * record echoes with nowhere to hang a per-row list.
+   *
+   * This is the path that matters most for the warning: `batchTransaction` is
+   * how the console's record form saves a master-detail record, so a
+   * `readonlyWhen`-locked field edited in that form was stripped server-side
+   * while the UI reported a plain success. The operation kind is taken from the
+   * originating op so the toast doesn't call an update a create.
+   */
+  private notifyBatchDroppedFields(
+    operations: BatchTransactionOperation[],
+    payload: unknown,
+  ): void {
+    const dropped = (payload as { droppedFields?: unknown } | null | undefined)?.droppedFields;
+    if (!Array.isArray(dropped) || dropped.length === 0) return;
+    for (const entry of dropped) {
+      if (!entry || typeof entry !== 'object') continue;
+      const e = entry as DroppedFieldsEvent & { index?: number };
+      if (!Array.isArray(e.fields) || e.fields.length === 0) continue;
+      const op = typeof e.index === 'number' ? operations[e.index] : undefined;
+      // `delete` never drops fields; anything unexpected reads as an update,
+      // which is the truthful default for a batch that echoed a strip.
+      const operation: 'create' | 'update' = (op?.action ?? 'create') === 'create' ? 'create' : 'update';
+      this.emitWriteWarning({
+        operation,
+        resource: e.object ?? op?.object ?? '',
+        ...(op?.id !== undefined && op?.id !== null ? { id: op.id } : {}),
+        droppedFields: [{ object: e.object, fields: e.fields, reason: e.reason }],
+      });
+    }
+  }
+
   /**
    * Subscribe to write-warning events (a create/update dropped caller-supplied
    * fields — #3431/#3455). Returns an unsubscribe function. The app shell uses
@@ -1225,6 +1260,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
       // dependency floor (framework #3271). No hand-rolled POST /api/v1/batch.
       const payload = await this.client.data.batchTransaction(operations);
       this.emitBatchMutations(operations, payload?.results);
+      this.notifyBatchDroppedFields(operations, payload);
       return payload;
     } catch (err) {
       // On a non-declaring backend, endpoint missing (404/405) or a runtime that

packages/data-objectstack/src/onWriteWarning.test.ts

@@ -94,6 +94,56 @@ describe('ObjectStackAdapter.onWriteWarning', () => {
     expect(events).toEqual([]);
   });
 
+  // ── cross-object batch (framework #3794) ─────────────────────────────────
+  //
+  // `batchTransaction` is the console record form's save path for a
+  // master-detail record, so this is where a `readonlyWhen` strip actually
+  // reaches a user editing a form. Its response tags each event with the index
+  // of the operation that produced it.
+
+  it('emits a write-warning per event on a cross-object batch, resolving the op', async () => {
+    const batchTransaction = vi.fn().mockResolvedValue({
+      results: [{ id: 'acc1' }, { id: 'inv1' }],
+      droppedFields: [
+        { object: 'invoice', fields: ['tax_rate'], reason: 'readonly_when', index: 1 },
+      ],
+    });
+    const ds = makeDS({ batchTransaction });
+    ds.atomicBatchCapability = true;
+    const events: WriteWarningEvent[] = [];
+    ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
+
+    await ds.batchTransaction([
+      { object: 'account', action: 'create', data: { name: 'Acme' } },
+      { object: 'invoice', action: 'update', id: 'inv1', data: { status: 'paid', tax_rate: 9 } },
+    ]);
+
+    expect(events).toEqual([
+      {
+        operation: 'update',
+        resource: 'invoice',
+        id: 'inv1',
+        droppedFields: [{ object: 'invoice', fields: ['tax_rate'], reason: 'readonly_when' }],
+      },
+    ]);
+  });
+
+  it('does NOT emit for a clean batch or a malformed droppedFields list', async () => {
+    const batchTransaction = vi
+      .fn()
+      .mockResolvedValueOnce({ results: [{ id: 'a' }] })
+      .mockResolvedValueOnce({ results: [{ id: 'a' }], droppedFields: [{ object: 'x', fields: [], index: 0 }] });
+    const ds = makeDS({ batchTransaction });
+    ds.atomicBatchCapability = true;
+    const events: WriteWarningEvent[] = [];
+    ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
+
+    await ds.batchTransaction([{ object: 'x', action: 'create', data: {} }]);
+    await ds.batchTransaction([{ object: 'x', action: 'create', data: {} }]);
+
+    expect(events).toEqual([]);
+  });
+
   it('stops delivering after unsubscribe', async () => {
     const create = vi.fn().mockResolvedValue({ record: { id: 'r1' }, droppedFields: ONE_DROP });
     const ds = makeDS({ create });