fix(app-shell): inspectors read and write the expression envelope (#3218) (#3228)

`HookSchema.condition` is `ExpressionInputSchema`, so a persisted hook carries
`{ dialect, source }` — the Hook inspector's guard box read a bare string only
and rendered empty, and `ConditionBuilder.emit` compiles only the rows on
screen, so the author's next edit replaced a guard they were never shown.

Read side converges on `conditionText` (the one reader #3216 settled on) via a
shared `expressionSource` / `writeExpressionSource` pair — no fourth
`typeof c === 'string'`. Write side follows the #3218 ruling: preserve
`dialect` and `meta`, replace `source`, DISCARD `ast` (derived from the old
source; `objectstack compile` refills it). With no prior envelope the commit
stays the bare-string shorthand, which the spec's pipe normalizes to
`{ dialect: 'cel', source }`.

Swept the rest of the family: action `visible` / `disabled`, the generic
SchemaForm condition widget (which put a literal `[object Object]` in the
editor), the object-validations rule `condition` plus its type-switch carry
(a third narrow read that dropped the guard), and the flow-edge inspector's
write, whose read #3216 had already fixed.

Fixtures are authored input fed through `HookSchema.parse`, never hand-written
envelopes, so they cannot drift from the spec.


Claude-Session: https://claude.ai/code/session_01PRJtkgUAaVG11FsJQbvZWA

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

Author: os-zhuang

.changeset/hook-inspector-reads-the-expression-envelope.md

@@ -0,0 +1,62 @@
+---
+"@object-ui/app-shell": minor
+---
+
+Inspectors read AND write the `{ dialect, source }` expression envelope (objectui#3218).
+
+The Hook inspector's "Run only when (optional CEL)" box rendered **empty** for a
+hook that had a guard. `HookSchema.condition` is `ExpressionInputSchema` — the
+same `ZodPipe` as `FlowEdgeSchema.condition` — so parsing `condition: 'amount > 10'`
+**rewrites it into** `{ dialect: 'cel', source: 'amount > 10' }`. The envelope is
+what a persisted hook carries; the inspector read `typeof draft.condition ===
+'string'` and fell through to `''`.
+
+An empty box is not a cosmetic defect here. `ConditionBuilder.emit` compiles only
+the rows currently on screen, so the author's next edit **replaced** a guard they
+were never shown (clearing it committed `condition: undefined`). Opening the
+panel is safe on its own — `onCommit` fires only on a real edit — but the empty
+box is what induces that edit.
+
+**Read.** Every one of these surfaces now goes through `conditionText`, the one
+reader objectui#3216 settled on, via a shared `expressionSource` /
+`writeExpressionSource` pair. No new `typeof c === 'string'` was written.
+
+**Write.** `source` was the only key the commit path preserved — everything else
+in the envelope was discarded, because the commit sent a bare string and the
+spec's pipe hardcodes `dialect: 'cel'`. Editing one character of a
+`dialect: 'cron'` or `dialect: 'template'` guard silently moved it to a different
+evaluation engine, and dropped `ast` and ADR-0089 `meta` (`rationale` /
+`generatedBy` — the keys AI-authored metadata fills and nobody restores by hand).
+An edit now:
+
+| key | behaviour |
+|:--|:--|
+| `dialect` | **preserved** |
+| `meta` | **preserved** |
+| `source` | replaced |
+| `ast` | **discarded** — it was compiled from the OLD source, so keeping it would leave the engine evaluating the old guard while the UI shows the new one. `objectstack compile` refills it, and `ExpressionSchema`'s `source \|\| ast` refinement still holds. |
+
+With no prior envelope to preserve, the commit stays the bare-string shorthand —
+which the spec's pipe normalizes to exactly `{ dialect: 'cel', source }`, so
+nothing is lost and plain-`string` predicate fields keep round-tripping as
+strings.
+
+Four surfaces were in this family, not one:
+
+- **Hook inspector** — `condition` (the reported defect).
+- **Action inspector** — `visible` and `disabled` (`boolean | ExpressionInput`),
+  same empty-box read.
+- **The generic SchemaForm condition widget** — every predicate-named field
+  (`visible` / `hidden` / `disabled` / `condition` / `predicate` / `*When`) routes
+  here, and it did `String(value)`: an envelope reached the editor as the literal
+  text `[object Object]`.
+- **Object validations panel** — the rule `condition`, plus a third narrow read
+  in the type switcher that dropped a persisted guard on the floor and left the
+  skeleton's never-firing `'false'` in its place. `ValidationRuleDraft.condition`
+  is now `ExpressionInput` instead of `string`.
+
+The flow-edge inspector's **write** is fixed the same way; objectui#3216 had
+converged only its read.
+
+Fixtures in the new tests are authored input fed through `HookSchema.parse` — no
+envelope is hand-written — so they cannot drift from the spec.

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

@@ -50,6 +50,7 @@ import { useObjectOptions } from '../previews/useObjectOptions';
 import { useObjectFields } from '../previews/useObjectFields';
 import { useMetaOptions } from '../previews/useMetaOptions';
 import { ConditionBuilder } from './ConditionBuilder';
+import { expressionSource, writeExpressionSource } from './expression-envelope';
 import { IconPickerWidget } from '../widgets';
 
 /* ─────────────── constants ─────────────── */
@@ -464,8 +465,11 @@ export function ActionDefaultInspector({
       {/* 6 ─ Conditions */}
       <div className="border-t pt-3 space-y-3">
         <SectionHeader title="Conditions" hint="No-code predicates over the record / user / ctx (compiled to CEL)." />
-        <ConditionBuilder label="Visible when" value={typeof draft.visible === 'string' ? (draft.visible as string) : ''} onCommit={(v) => onPatch({ visible: v || undefined })} objectName={objectName} disabled={readOnly} />
-        <ConditionBuilder label="Disabled when" value={typeof draft.disabled === 'string' ? (draft.disabled as string) : ''} onCommit={(v) => onPatch({ disabled: v || undefined })} objectName={objectName} disabled={readOnly} />
+        {/* Both are `ExpressionInputSchema` in the spec (`disabled` as
+            `boolean | ExpressionInput`), so a persisted action carries the
+            ADR-0089 envelope — same read/write pair as the hook guard (#3218). */}
+        <ConditionBuilder label="Visible when" value={expressionSource(draft.visible)} onCommit={(v) => onPatch({ visible: writeExpressionSource(draft.visible, v) })} objectName={objectName} disabled={readOnly} />
+        <ConditionBuilder label="Disabled when" value={expressionSource(draft.disabled)} onCommit={(v) => onPatch({ disabled: writeExpressionSource(draft.disabled, v) })} objectName={objectName} disabled={readOnly} />
       </div>
 
       {/* 7 ─ AI exposure */}

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

@@ -32,6 +32,7 @@ import { validateExpressionClient } from './expression-validate';
 import { useFlowScope } from './useFlowScope';
 import { VariableTextInput } from './VariableTextInput';
 import { findUnknownRefs, scopeRoots, describeUnknownRefs } from './flow-ref-check';
+import { writeExpressionSource } from './expression-envelope';
 import type { ExpressionInput } from '@objectstack/spec/shared';
 
 /**
@@ -232,11 +233,16 @@ export function FlowEdgeInspector({ selection, draft, onPatch, onClearSelection,
       />
       <div className="space-y-1">
         <Label className="text-xs text-muted-foreground">{t('engine.inspector.flowEdge.condition', locale)}</Label>
+        {/* #3216 converged the READ on `conditionText`; the write stayed a
+            bare string, which the spec's pipe would have re-stamped as
+            `dialect: 'cel'` — silently swapping the engine of a `cron` /
+            `template` guard and dropping its `ast` / `meta`. Same rule as the
+            hook guard now (#3218). */}
         <VariableTextInput
           mode="expression"
           mono
           value={conditionText(edge.condition) ?? ''}
-          onValueChange={(v) => patchEdge({ condition: v || undefined })}
+          onValueChange={(v) => patchEdge({ condition: writeExpressionSource(edge.condition, v) })}
           groups={scopeGroups}
           placeholder={t('engine.inspector.flowEdge.conditionHint', locale)}
           disabled={readOnly || isDefault}

packages/app-shell/src/views/metadata-admin/inspectors/HookDefaultInspector.condition.test.tsx

@@ -0,0 +1,172 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * objectui#3218 — the Hook inspector's "Run only when" guard must read AND
+ * write the ADR-0089 expression envelope.
+ *
+ * `HookSchema.condition` is `ExpressionInputSchema`, the same pipe
+ * `FlowEdgeSchema.condition` uses: a bare authored string is NORMALIZED at
+ * parse time into `{ dialect: 'cel', source }`, so the envelope is what a
+ * persisted hook actually carries. The inspector used to read
+ * `typeof draft.condition === 'string'`, so a persisted guard rendered EMPTY —
+ * and because `ConditionBuilder.emit` compiles only the rows currently on
+ * screen, the author's next edit REPLACED a guard they were never shown.
+ *
+ * FIXTURE DISCIPLINE (objectui#3216's method): no envelope is hand-written
+ * here. Every fixture is the AUTHORED input fed through `HookSchema.parse`, so
+ * a fixture cannot drift from the spec — if the spec stops normalizing, these
+ * tests change shape with it instead of quietly testing a shape nothing emits.
+ *
+ * The write rule (issue ruling, option B) is what the edit cases pin:
+ *
+ *   | key       | when `source` is edited                                |
+ *   |:----------|:-------------------------------------------------------|
+ *   | `dialect` | PRESERVED (only a value with no prior envelope is 'cel')|
+ *   | `meta`    | PRESERVED (ADR-0089 rationale / generatedBy)            |
+ *   | `source`  | replaced                                               |
+ *   | `ast`     | DISCARDED — derived from the OLD source                |
+ */
+
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, screen, fireEvent, cleanup } from '@testing-library/react';
+import { HookSchema } from '@objectstack/spec/data';
+import { HookDefaultInspector } from './HookDefaultInspector';
+
+afterEach(cleanup);
+
+/**
+ * Author a hook the way a user does, parse it with the spec, and hand the
+ * RESULT to the inspector — exactly what the metadata editor loads.
+ */
+function hookDraft(condition: unknown): Record<string, unknown> {
+  return HookSchema.parse({
+    name: 'guard_hook',
+    // '*' keeps ConditionBuilder in raw/no-catalog mode: the guard's read and
+    // write are what is under test, not the field picker's network fetch.
+    object: '*',
+    events: ['beforeInsert'],
+    handler: 'guard_fn',
+    condition,
+  }) as unknown as Record<string, unknown>;
+}
+
+function renderInspector(draft: Record<string, unknown>, onPatch = vi.fn()) {
+  render(
+    <HookDefaultInspector
+      type="hook"
+      name="guard_hook"
+      draft={draft}
+      onPatch={onPatch}
+      readOnly={false}
+      locale={'en-US' as never}
+    />,
+  );
+  return onPatch;
+}
+
+/** The no-code builder's value box for the single parsed row. */
+const rowValueInput = () => screen.getByPlaceholderText('value') as HTMLInputElement;
+/**
+ * The raw-expression editor. CelPredicateField renders a combobox TEXTAREA
+ * (autocomplete host); the panel's Radix selects are combobox buttons.
+ */
+const rawEditor = () =>
+  screen.getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement;
+
+describe('HookDefaultInspector — condition envelope (#3218)', () => {
+  it('renders the `source` of an envelope condition instead of an empty box', () => {
+    const draft = hookDraft('amount > 10');
+    // Pin what the platform actually stores, so this test fails loudly if the
+    // spec ever stops normalizing rather than passing on a stale assumption.
+    expect(draft.condition).toEqual({ dialect: 'cel', source: 'amount > 10' });
+
+    renderInspector(draft);
+
+    // The guard is visible: the no-code builder adopted `amount > 10`, so its
+    // value box carries `10` and the compiled preview echoes the whole guard.
+    expect(rowValueInput().value).toBe('10');
+    expect(screen.getByText('amount > 10')).toBeInTheDocument();
+  });
+
+  it('preserves `dialect` and `meta` verbatim across an edit', () => {
+    const draft = hookDraft({
+      dialect: 'cel',
+      source: 'amount > 10',
+      meta: { rationale: 'Only large deals need approval', generatedBy: 'agent:deal-guard' },
+    });
+    const onPatch = renderInspector(draft);
+
+    fireEvent.change(rowValueInput(), { target: { value: '20' } });
+
+    expect(onPatch).toHaveBeenCalledWith({
+      condition: {
+        dialect: 'cel',
+        source: 'amount > 20',
+        meta: { rationale: 'Only large deals need approval', generatedBy: 'agent:deal-guard' },
+      },
+    });
+  });
+
+  it('DISCARDS a stale `ast` on edit (it was compiled from the old source)', () => {
+    const draft = hookDraft({
+      dialect: 'cel',
+      source: 'amount > 10',
+      ast: { op: '>', left: 'amount', right: 10 },
+    });
+    expect((draft.condition as Record<string, unknown>).ast).toBeDefined();
+
+    const onPatch = renderInspector(draft);
+    fireEvent.change(rowValueInput(), { target: { value: '20' } });
+
+    const patched = onPatch.mock.calls.at(-1)![0].condition as Record<string, unknown>;
+    expect(patched.source).toBe('amount > 20');
+    // Keeping it would leave the engine evaluating the OLD guard while the UI
+    // shows the new one. `objectstack compile` refills it.
+    expect(patched).not.toHaveProperty('ast');
+    // Dropping `ast` is safe: `source` is present, so ExpressionSchema's
+    // "one of source | ast" refinement still holds.
+    expect(HookSchema.parse({ ...draft, condition: patched }).condition).toEqual({
+      dialect: 'cel',
+      source: 'amount > 20',
+    });
+  });
+
+  it('keeps a `template` guard on the template dialect (the A/B dividing line)', () => {
+    const draft = hookDraft({ dialect: 'template', source: 'Hello {{record.name}}' });
+    const onPatch = renderInspector(draft);
+
+    // Not simple-CEL, so the builder opens the raw expression editor — which
+    // is also proof the envelope's `source` reached the control.
+    expect(rawEditor().value).toBe('Hello {{record.name}}');
+
+    fireEvent.change(rawEditor(), { target: { value: 'Hello {{record.title}}' } });
+
+    // Committing a bare string here (option A) would have let the spec's pipe
+    // rewrite this guard to `dialect: 'cel'` — a different evaluation engine
+    // for an author who believes they only retyped the text.
+    expect(onPatch).toHaveBeenCalledWith({
+      condition: { dialect: 'template', source: 'Hello {{record.title}}' },
+    });
+  });
+
+  it('still clears the guard to `undefined` when the author empties it', () => {
+    const draft = hookDraft('amount > 10');
+    const onPatch = renderInspector(draft);
+
+    fireEvent.click(screen.getByLabelText('Remove condition'));
+
+    expect(onPatch).toHaveBeenCalledWith({ condition: undefined });
+  });
+
+  it('writes the bare-string shorthand when there was no prior envelope', () => {
+    // A hook authored without a guard: nothing to preserve, so the shorthand
+    // (which the spec's pipe normalizes to `dialect: 'cel'`) is what is sent.
+    const draft = hookDraft(undefined);
+    expect(draft.condition).toBeUndefined();
+
+    const onPatch = renderInspector(draft);
+    fireEvent.click(screen.getByText('Add condition'));
+    // A subject-less row compiles to '', i.e. still no guard.
+    expect(onPatch).toHaveBeenLastCalledWith({ condition: undefined });
+  });
+});

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

@@ -37,6 +37,7 @@ import {
 } from './_shared';
 import { useObjectOptions } from '../previews/useObjectOptions';
 import { ConditionBuilder } from './ConditionBuilder';
+import { expressionSource, writeExpressionSource } from './expression-envelope';
 
 /* ─────────────── constants ─────────────── */
 
@@ -251,10 +252,14 @@ export function HookDefaultInspector({
             <InspectorCheckboxField label="Run asynchronously (after commit)" value={draft.async === true} onCommit={(v) => onPatch({ async: v })} disabled={readOnly} />
           </div>
         </div>
+        {/* `HookSchema.condition` is `ExpressionInputSchema`: a persisted hook
+            carries the ADR-0089 envelope, not the authored string. Read and
+            write it through the shared pair so the guard is visible and an
+            edit cannot rewrite its dialect or drop its `meta` (#3218). */}
         <ConditionBuilder
           label="Run only when (optional CEL)"
-          value={typeof draft.condition === 'string' ? (draft.condition as string) : ''}
-          onCommit={(v) => onPatch({ condition: v || undefined })}
+          value={expressionSource(draft.condition)}
+          onCommit={(v) => onPatch({ condition: writeExpressionSource(draft.condition, v) })}
           objectName={conditionObject}
           disabled={readOnly}
         />