fix(app-shell): declare the spec's escalation.enabled default in the flow-node inspector

.changeset/6620-escalation-enabled-default.md

@@ -0,0 +1,37 @@
+---
+'@object-ui/app-shell': patch
+---
+
+The flow-node inspector now declares the spec's default for
+`escalation.enabled`, so approval nodes that omit the key render as the
+escalating nodes they are (objectui#6620).
+
+`FLOW_NODE_CONFIG`'s approval group declared `defaultValue: 'false'` for
+`escalation.enabled`, while the installed `@objectstack/spec` (17.3.0) defaults
+the key to `true` — `ApprovalEscalationSchema.safeParse({ timeoutHours: 24 })`
+returns `enabled: true`. An approval node whose escalation block omits the key
+therefore escalates at runtime, while the designer drew the SLA-escalation
+toggle OFF and hid all four sub-fields behind it (`timeoutHours`, `action`,
+`escalateTo`, `notifySubmitter`). The inspector said "no escalation" about a
+node that escalates, and the author had no way to see or edit the timeout that
+was live.
+
+`defaultValue` has two read sites, so the one-line flip repairs both: it is what
+`controllerAdmits` resolves an unset `showWhen` controller against (the gating
+of those four sub-fields) and, since the boolean control learned to seed itself,
+what the rendered toggle shows. The online half of the same form was already
+correct — a backend publishing the approval `configSchema` sends `default: true`,
+which `json-schema-to-fields` turns into `defaultValue: 'true'` — so offline and
+online rendered the same node from two different claims about the spec.
+
+An explicitly stored `escalation.enabled: false` is unchanged: it still beats the
+declared default and still hides every sibling.
+
+The reconciliation assertion in `flow-node-config.spec-reconciliation.test.ts`
+now covers the whole escalation block rather than `notifySubmitter` alone, and
+derives every expected value from the installed `ApprovalEscalationSchema`
+instead of pinning literals. This is the substance of the fix as much as the flip
+is: the previous tripwire for this key read only the hand-written table, so a
+spec bump could never redden it and the divergence went live unnoticed. A
+vacuity guard fails if the spec stops materialising defaults for the block, so
+the ledger cannot quietly become a comparison against nothing.

packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx

@@ -224,12 +224,20 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale,
         //    renderer-side alias for off-spec metadata (AGENTS.md #0.1).
         //
         // ⛔ Nothing is rendered NAMING the default — no "(default)" caption,
-        // no help line. Of the two boolean fields the offline table declares a
-        // default for, one (`escalation.enabled`) declares the OPPOSITE of what
-        // the installed spec applies to an omitted key (objectui#6620), so any
-        // caption asserting "this is the declared default" would ship that
-        // wrong claim to authors in words. The seed alone leaves that field
-        // rendering byte-identically to before; a caption would not. Pinned by
+        // no help line. That is triage's arm-A ruling and stands on its own.
+        //
+        // ⚑ Its original SECONDARY argument has been discharged and must not be
+        // cited again: `escalation.enabled` used to declare the OPPOSITE of what
+        // the installed spec applies to an omitted key, so a caption would have
+        // shipped a wrong claim to authors in words. objectui#6620 fixed the
+        // declaration and, from this seed, that field now draws CHECKED — it no
+        // longer renders byte-identically to before, and the argument that the
+        // seed is the safe half of the change no longer applies to it. Both
+        // boolean fields the offline table declares a default for now agree with
+        // the spec, and `flow-node-config.spec-reconciliation.test.ts` keeps the
+        // whole escalation block reconciled against the installed
+        // `ApprovalEscalationSchema`, so a future divergence reddens there rather
+        // than being absorbed by a rendering decision here. Pinned by
         // `FlowNodeInspector.declaredDefault.test.tsx`'s #6620 case.
         const checked = isUnsetFieldValue(value) ? field.defaultValue === 'true' : value === true;
         return (

packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx

@@ -77,6 +77,12 @@ vi.mock('../previews/useObjectFields', () => ({
 import { FlowNodeInspector } from './FlowNodeInspector';
 import { fieldsForNodeType, FLOW_NODE_TYPE_OPTIONS } from './flow-node-config';
 import type { MetadataSelection } from '../preview-registry';
+// The runtime's own answer for an omitted `escalation` key. Imported so the
+// #6620 rows below compare the RENDERED control against the installed contract
+// instead of against a literal this file would then own a second copy of.
+// ⛔ The subpath is load bearing: `ApprovalEscalationSchema` is NOT on the
+// package root, where it reads `undefined` and any `.parse` on it throws.
+import { ApprovalEscalationSchema } from '@objectstack/spec/automation';
 
 /* ── The `meta/*` double (objectui#7307) ───────────────────────────────
  * `FlowNodeInspector` renders `FlowReferenceField` for every reference-kind key
@@ -276,25 +282,36 @@ describe('boolean: a declared defaultValue seeds the control (objectui#8451, arm
     ).toBe(false);
   });
 
-  it('boolean: the #6620-wrong declaration ships NO worded claim (objectui#6620)', () => {
-    // `escalation.enabled` is the one field in the offline table whose declared
-    // default contradicts what the installed spec applies to an omitted key.
-    // This card ships the SEED and no caption, which is why: the seed renders
-    // that field exactly as it rendered before (unchecked — the declaration
-    // says 'false'), so no new claim about the declaration reaches the author,
-    // while a "(default)" caption would have asserted the wrong one in words.
+  it('boolean: the gate seeds from the SPEC default, still with no worded claim (objectui#6620)', () => {
+    // Was 'the #6620-wrong declaration ships NO worded claim'. `escalation.enabled`
+    // used to be the one field in the offline table whose declared default
+    // contradicted what the installed spec applies to an omitted key, so this row
+    // pinned that the seed shipped no caption asserting the wrong default in words.
+    // objectui#6620 fixed the declaration; the row now pins the repaired half —
+    // the control the author sees agrees with the runtime — and the no-caption rule
+    // still holds, now over a declaration that is right.
     //
-    // ⚠️ Deliberately does NOT compare the declaration against the spec. That
-    // comparison is objectui#6620's tripwire, held disarmed on purpose in
-    // `flow-node-config.spec-reconciliation.test.ts`; arming it here would
-    // discharge an on-hold card from an unrelated PR.
+    // ⭐ The expectation is DERIVED from the installed spec, never the literal
+    // 'true'. A literal would restate the very claim that drifted: it passes just
+    // as happily on the next upstream flip, which is how #6620 stayed invisible.
+    const specEnabled = ApprovalEscalationSchema.safeParse({ timeoutHours: 24 });
+    expect(specEnabled.success, 'a minimal escalation block must parse').toBe(true);
+    const specDefault = (specEnabled.data as { enabled?: unknown } | undefined)?.enabled;
+    // Vacuity guard: without it, a spec that stopped materialising the key would
+    // turn both sides into `undefined` and the comparison into a tautology.
+    expect(typeof specDefault, 'the spec still materialises `enabled` from an omitted key').toBe('boolean');
+
     const gate = fieldsForNodeType('approval').find((f) => f.id === 'escalation.enabled');
-    expect(gate?.defaultValue, 'the gate declares a default at all').toBe('false');
+    // Defaults are strings in this table — the spelling `controllerAdmits` compares.
+    expect(gate?.defaultValue, 'the table declares the default the spec applies').toBe(String(specDefault));
 
     renderInspector(draftWith('approval', { config: { escalation: { timeoutHours: 24 } } }));
     const box = checkbox('SLA escalation');
     expect(box, 'the gate control is on screen').not.toBeNull();
-    expect(box!.checked, 'and it renders as it always did — unchecked').toBe(false);
+    expect(
+      box!.checked,
+      'and the RENDERED toggle carries the spec default — the defect was that it did not',
+    ).toBe(specDefault);
     expect(
       box!.closest('label')?.textContent,
       'the control carries its label and nothing else — no caption naming a default',
@@ -329,11 +346,14 @@ describe('the online writer of defaultValue hits the same dead end', () => {
 describe('the one read site: a declared default on a showWhen CONTROLLER changes visibility', () => {
   /**
    * `controllerAdmits` resolves an UNSET controller through its `defaultValue`.
-   * The hand-written approval group declares `escalation.enabled: 'false'`, so
-   * the offline table cannot demonstrate the effect (an absent default and a
-   * 'false' default both hide the group). The engine-published schema can: the
-   * spec's own `ApprovalEscalationSchema` defaults `enabled` to TRUE, and
-   * `json-schema-to-fields` carries that onto the gate field.
+   *
+   * The hand-written approval group used to declare `escalation.enabled: 'false'`,
+   * so the offline table could not demonstrate the effect at all — an absent
+   * default and a 'false' default both hide the group, which is precisely why the
+   * divergence survived: the two writers of this property answered the same node
+   * differently and only the engine-published one was pinned. objectui#6620 made
+   * the table declare the spec's `.default(true)`, so BOTH writers are exercised
+   * here now, and the offline row below is the one that changed.
    */
   const escalationSchema = {
     type: 'object',
@@ -374,15 +394,45 @@ describe('the one read site: a declared default on a showWhen CONTROLLER changes
     ).toBeNull();
   });
 
-  it('the offline table hides the same siblings, because it declares the gate false', () => {
-    // Recorded, not endorsed: the hand-written 'false' contradicts the spec's
-    // `.default(true)`. That divergence is objectui#6620's subject, not this
-    // card's; pinned here only so a change to either side is visible.
+  it('the offline table REVEALS the same siblings — it declares the gate true (objectui#6620)', () => {
+    // Was 'the offline table hides the same siblings, because it declares the gate
+    // false' — recorded there, explicitly not endorsed, because the hand-written
+    // 'false' contradicted the spec's `.default(true)`. objectui#6620 removed the
+    // divergence, so the two writers now answer this node identically. The row is
+    // updated to the new truth rather than worked around: it exists to make a
+    // change to either side visible, and this was that change.
     renderInspector(draftWith('approval', { config: { escalation: { timeoutHours: 24 } } }));
+    const sibling = checkbox('Notify submitter');
     expect(
-      checkbox('Notify submitter'),
-      'offline, the same node hides the sibling the online schema reveals',
-    ).toBeNull();
+      sibling,
+      'offline, the same node reveals the sibling the online schema reveals',
+    ).not.toBeNull();
+    expect(
+      sibling!.checked,
+      'and the offline table seeds it from its own declared default too',
+    ).toBe(true);
+  });
+
+  /**
+   * NON-REGRESSION (objectui#6620). Derived from a plausible WRONG FIX rather
+   * than from the bug: revealing the sub-fields unconditionally — dropping the
+   * `showWhen` gates, or making `controllerAdmits` fall through to `true` —
+   * satisfies "a node that omits the key now reveals" completely, while
+   * destroying the author's ability to turn escalation OFF.
+   *
+   * So the stored `false` must still beat the declared default, and must still
+   * hide EVERY sibling, not just the first. Four keys, named individually: an
+   * assertion over one of them passes on a fix that leaks the other three.
+   */
+  it('a node that explicitly stores `enabled: false` still hides every sibling', () => {
+    renderInspector(draftWith('approval', { config: { escalation: { enabled: false } } }));
+    expect(checkbox('SLA escalation')!.checked, 'the stored false beats the declared true').toBe(false);
+    for (const label of ['Timeout (hours)', 'On timeout', 'Escalate to', 'Notify submitter']) {
+      expect(
+        screen.queryByText(label),
+        `'${label}' must stay hidden while the author has switched escalation off`,
+      ).toBeNull();
+    }
   });
 });
 

packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts

@@ -62,20 +62,27 @@ describe('inactiveRetainedKind — the escalation instance the card measured', (
     for (const f of ungated) expect(inactiveRetainedKind(f, node, fields)).toBeNull();
   });
 
-  it('an absent controller value resolves through the spec defaultValue, not to "off"', () => {
-    // `escalation.enabled` declares defaultValue 'false', so an omitted key is
-    // off — and a stored dependent under it is therefore retained-but-inactive.
+  it('an absent controller value resolves through the spec defaultValue — now ON, so the dependent is LIVE', () => {
+    // `escalation.enabled` declares defaultValue 'true', mirroring the installed
+    // spec's `.default(true)`: an omitted key means ENABLED, so a stored dependent
+    // under it is live config, not retained-but-inert.
     //
-    // ⚠️ Coupled to objectui#6620 ON PURPOSE. That card is the mirror defect:
-    // `@objectstack/spec` flipped `ApprovalEscalation.enabled` to `default(true)`
-    // upstream, so once this repo consumes a spec release carrying the flip, this
-    // descriptor's `defaultValue: 'false'` becomes wrong and must follow. When it
-    // does, THIS assertion flips to `toBeNull()` — an omitted key will mean ON,
-    // and a stored dependent under it is live, not retained. Measured here on
-    // 2026-08-29: installed spec is 17.2.0, still `.default(false)`, so the two
-    // agree and #6620 is latent. The failure is the intended signal, not a break.
+    // This assertion flipped from `'controller-off'` to `toBeNull()` when
+    // objectui#6620 made the table follow the spec — exactly the flip the previous
+    // revision of this comment predicted, and the intended signal rather than a
+    // break. (Measured on the spec installed then: 17.2.0 / `.default(false)`.)
+    //
+    // ⚠️ It still reads only the TABLE, so it cannot see a spec bump on its own.
+    // The declaration ↔ installed-spec comparison that CAN detect the next
+    // divergence lives in `flow-node-config.spec-reconciliation.test.ts`; #6620's
+    // root cause was that no such comparison existed for this key.
     const node = { id: 'a', type: 'approval', config: { escalation: { timeoutHours: 24 } } };
-    expect(inactiveRetainedKind(timeout(), node, fields)).toBe('controller-off');
+    expect(inactiveRetainedKind(timeout(), node, fields)).toBeNull();
+    // Lit control on the same node shape — the ONLY difference is the stored gate.
+    // A predicate that had simply stopped flagging anything fails here, so the
+    // `toBeNull()` above is a measurement rather than an absence.
+    const gateOff = { id: 'a', type: 'approval', config: { escalation: { enabled: false, timeoutHours: 24 } } };
+    expect(inactiveRetainedKind(timeout(), gateOff, fields)).toBe('controller-off');
   });
 
   it('flags every dependent in the group, not just the first', () => {