fix(app-shell): flow simulator evaluates a { dialect, source } edge guard (#3216) (#3217)

* fix(app-shell): flow simulator evaluates a `{ dialect, source }` edge guard (#3216)

Two readers in `previews/simulator/` each hand-rolled `typeof c === 'string'`
and returned `undefined` for anything else, so a decision out-edge guarded by
the ADR-0089 expression envelope was reported as `Branch has no condition.`
and skipped — while the engine evaluates it normally at run time. That
envelope is not an exotic spelling: `ExpressionInputSchema` is a `ZodPipe`
that rewrites an authored guard string INTO `{ dialect: 'cel', source }`, and
`FlowEdgeSchema.condition` is that schema, so the shape the simulator could
not read is the shape the platform produces.

Both readers now go through `conditionText` — the one reader every other
consumer of the field already used (canvas labels, `FlowEdgeInspector`, the
Branches<->edges reconciliation) — so "how an edge guard is read" has exactly
one answer in this repo. `validateFlowDraft`'s "no default branch" warning
follows: a decision whose guards are envelopes was silently exempt from it.

`SimEdge.condition` mirrors the spec's `ExpressionInput` by importing it —
the last copy of the restatement objectui#3202 removed from
`FlowDesignerEdge` — pinned by compile-time assertions in the project CI
type-checks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRJtkgUAaVG11FsJQbvZWA

* test(app-shell): pin "a persisted edge is a SimEdge", and stop overclaiming (#3216)

The behavioural suite's `specEdge` helper annotates its return as `SimEdge`,
and its JSDoc called that "a compile-time proof". It is not one here: the
package tsconfig excludes `**/*.test.ts`, app-shell's test tree is still
TEST_DEBT in the type-check ratchet, and vitest erases types — so no `tsc`
run reads that annotation. A comment claiming enforcement that nothing
performs is the objectui#3009 / objectui#3181 failure mode, in miniature.

The claim moves to where it IS compiled: `flow-sim-edge.types.test.ts`, in
`tsconfig.typetests.json`, now asserts `z.infer<typeof FlowEdgeSchema>`
extends `SimEdge` — an edge the server hands back is a thing the simulator
accepts, with no reconciliation and no cast. The helper's JSDoc says plainly
that its annotation is a statement, not a check, and points at the pin.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PRJtkgUAaVG11FsJQbvZWA

---------

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

Author: os-zhuang

.changeset/flow-simulator-reads-the-expression-envelope.md

@@ -0,0 +1,51 @@
+---
+"@object-ui/app-shell": minor
+---
+
+Flow simulator evaluates an edge guard stored as `{ dialect, source }` (objectui#3216).
+
+A decision's out-edge whose guard is the ADR-0089 expression envelope — say
+`{ dialect: 'cel', source: 'amount > 10' }` — was reported on the debug timeline
+as `Branch has no condition.` and skipped. With `amount = 20` the simulation fell
+through to the default branch (or dead-ended with "No branch matched"), while the
+engine takes that branch at run time. A designer-time debugger that shows a
+different route than the runtime is worse than no debugger, and it is the one
+thing the simulator's own contract forbids: *never silently simulate semantics
+that differ from the runtime*.
+
+The envelope is not an exotic spelling. `ExpressionInputSchema` in
+`@objectstack/spec` is a `ZodPipe`: parsing `condition: 'amount > 10'` **rewrites
+it into** `{ dialect: 'cel', source: 'amount > 10' }`, and `FlowEdgeSchema.condition`
+is that schema. So the shape the simulator could not read is the shape the
+platform itself produces for every authored guard.
+
+Two readers in `previews/simulator/` each hand-rolled `typeof c === 'string' ? c :
+undefined`, while every other consumer in this repo already accepted both
+spellings — `conditionText` (canvas labels, `FlowEdgeInspector`, the Branches↔edges
+reconciliation) and `validateExpressionClient` (the Problems panel). Both now go
+through `conditionText`, so "how an edge guard is read" has exactly one answer.
+Its JSDoc says so, because a fifth hand-rolled copy brings this class of bug
+straight back.
+
+Two behaviours change, both toward the runtime:
+
+- **Decision routing** — a branch guarded by an envelope is now evaluated, and
+  selected when true. The timeline shows the CEL source it ran instead of
+  "no condition". An envelope carrying only a compiled `ast` and no `source`
+  (spec phase M9.2) still reports "no condition": there is nothing to evaluate,
+  and the simulator says so rather than faking a result.
+- **Preflight diagnostics** — `validateFlowDraft` warns that a decision has no
+  default branch when *every* out-edge is guarded. A decision whose guards were
+  envelopes was silently exempt from that warning; it is exactly as able to
+  dead-end, so the warning now appears in the Problems panel and the canvas
+  banner for those flows too.
+
+**Type change:** `SimEdge.condition` is now the spec's `ExpressionInput`,
+**imported** rather than restated — the last copy of the restatement objectui#3202
+removed from `FlowDesignerEdge`. `string | { source?: string }` was wrong in both
+directions at once: too wide, since it describes a `dialect`-less envelope the
+server rejects; too narrow, since excess-property checking then refused the
+canonical envelope written as a literal (`'dialect' does not exist in type
+'{ source?: string }'`) — the one shape a persisted flow actually carries was the
+one shape you could not write down. Compile-time assertions pin it in
+`tsconfig.typetests.json`, the project CI actually type-checks.

packages/app-shell/src/views/metadata-admin/previews/flow-canvas-layout.ts

@@ -457,7 +457,26 @@ export function edgeKey(edge: FlowDesignerEdge, index: number): string {
   return edge.id || `${edge.source}->${edge.target}#${index}`;
 }
 
-/** Human-readable condition text for an edge's optional guard. */
+/**
+ * The CEL source of an edge's optional guard — **the** reader for that field.
+ *
+ * Both authoring spellings resolve here: the bare string, and the ADR-0089
+ * envelope that the spec's `ExpressionInputSchema` normalizes every authored
+ * string INTO at parse time (so the envelope is what a persisted flow carries).
+ * An envelope with only a compiled `ast` and no `source` (spec phase M9.2) has
+ * no readable source yet, and says so rather than inventing text.
+ *
+ * Every consumer of `condition` goes through this function — canvas labels and
+ * the inspector, the Branches↔edges reconciliation in
+ * `inspectors/flow-decision-edges.ts`, AND the simulator (both its decision
+ * routing and its preflight diagnostics). That is deliberate and load-bearing:
+ * "how an edge guard is read" had four hand-rolled answers, two of which
+ * (`simulator/flow-simulator.ts`, `simulator/flow-sim-validate.ts`) accepted
+ * only the bare string and so reported a spec-canonical envelope as "no
+ * condition" — the simulator skipped a branch the engine takes (objectui#3216).
+ * Add a fifth spelling and that class of bug comes straight back; call this
+ * instead.
+ */
 export function conditionText(c: FlowDesignerEdge['condition']): string | undefined {
   if (!c) return undefined;
   if (typeof c === 'string') return c;

packages/app-shell/src/views/metadata-admin/previews/simulator/__tests__/flow-sim-edge.types.test.ts

@@ -0,0 +1,109 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * `SimEdge.condition` is the spec's expression envelope — pinned at compile
+ * time (objectui#3216).
+ *
+ * This was the LAST copy of the restatement objectui#3202 removed from
+ * `FlowDesignerEdge`: `string | { source?: string }`, a spelling wrong in both
+ * directions at once. Too WIDE, because it describes an envelope with no
+ * `dialect` — a shape the server's `FlowEdgeSchema` rejects outright. Too
+ * NARROW, because excess-property checking then refuses the CANONICAL envelope
+ * written as a literal: `condition: { dialect: 'cel', source }` fails with
+ * "'dialect' does not exist in type '{ source?: string }'", so the one shape a
+ * persisted flow actually carries was the one shape you could not write down.
+ * Both errors come from the same act — writing the shape out by hand instead of
+ * importing it — and correcting it by hand only moves the drift to the next
+ * omitted member (`ast`, `meta`).
+ *
+ * The over-wide half is not academic: the simulator's readers took exactly the
+ * branch that type invited (`typeof c === 'string'`, everything else
+ * `undefined`) and skipped envelope guards the engine evaluates. The assertions
+ * below make the local type incapable of describing either error again.
+ *
+ * The assertions ARE the file, so it is listed in
+ * `packages/app-shell/tsconfig.typetests.json`: the package's build tsconfig
+ * excludes `**\/*.test.ts` and vitest erases types before running
+ * (objectui#3181), so unlisted they would be commentary, not a check.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { FlowEdgeSchema } from '@objectstack/spec/automation';
+import type { z } from 'zod';
+import type { ExpressionInput } from '@objectstack/spec/shared';
+import type { SimEdge } from '../flow-sim-types';
+import type { FlowDesignerEdge } from '../../flow-canvas-layout';
+
+type Assert< T extends true > = T;
+type Extends< A, B > = [A] extends [B] ? true : false;
+type IsAny< T > = 0 extends 1 & T ? true : false;
+type Equal< A, B > = (< T >() => T extends A ? 1 : 2) extends < T >() => T extends B ? 1 : 2
+  ? true
+  : false;
+
+type Condition = NonNullable< SimEdge['condition'] >;
+/** An edge as the server PERSISTS it — `FlowEdgeSchema`'s parsed output. */
+type PersistedEdge = z.infer< typeof FlowEdgeSchema >;
+
+describe('SimEdge.condition mirrors the spec ExpressionInput', () => {
+  it('is pinned at compile time', () => {
+    // Guard against the probe lying: were either side `any`, every
+    // assignability assertion below would pass while proving nothing.
+    type _SpecNotAny = Assert< Equal< IsAny< ExpressionInput >, false > >;
+    type _LocalNotAny = Assert< Equal< IsAny< Condition >, false > >;
+
+    // Not "compatible with" — the SAME type. Restating it is how they drift.
+    type _IsExactlyTheSpecType = Assert< Equal< Condition, ExpressionInput > >;
+
+    // …and therefore the same type the designer canvas carries, so an edge can
+    // cross from the canvas into the simulator with nothing to reconcile.
+    type _AgreesWithTheCanvas = Assert< Equal< Condition, NonNullable< FlowDesignerEdge['condition'] > > >;
+
+    // The authored shorthand (a bare CEL string) stays authorable…
+    type _StringIsACondition = Assert< Extends< string, Condition > >;
+    // …as does the envelope the spec canonicalises that string INTO — the form
+    // the two readers used to report as "no condition".
+    type _EnvelopeIsACondition = Assert< Extends< { dialect: 'cel'; source: string }, Condition > >;
+    // …including the optional authorship `meta` the old restatement dropped.
+    type _MetaIsACondition = Assert<
+      Extends< { dialect: 'cel'; source: string; meta: { generatedBy: string } }, Condition >
+    >;
+
+    // The over-wide half of the regression: a `dialect`-less envelope is no
+    // longer expressible. `FlowEdgeSchema` rejects it, so this type must too.
+    type _DialectlessRejected = Assert< Equal< Extends< { source: string }, Condition >, false > >;
+    // And `dialect` is closed over the spec's three dialects.
+    type _DialectIsClosed = Assert< Equal< Extends< { dialect: 'sql'; source: string }, Condition >, false > >;
+
+    // The whole point, stated end to end: an edge the SERVER hands back is a
+    // thing the simulator accepts, with no reconciliation and no cast. That is
+    // what objectui#3216 was not — the simulator's own input type described a
+    // guard shape the persisted edge never has.
+    type _PersistedEdgeNotAny = Assert< Equal< IsAny< PersistedEdge >, false > >;
+    type _PersistedEdgeIsASimEdge = Assert< Extends< PersistedEdge, SimEdge > >;
+
+    expect(true).toBe(true);
+  });
+
+  it('accepts the canonical envelope written INLINE — the half `Extends` cannot see', () => {
+    // The assertions above compare types; this one exercises excess-property
+    // checking, which is where the old restatement failed in the NARROW
+    // direction. Under `{ source?: string }` both consts below are compile
+    // errors ("'dialect' does not exist in…"), which is why an edge fixture in
+    // this repo could never spell the shape the spec canonicalises guards into.
+    const guarded: SimEdge = {
+      source: 'd',
+      target: 'hi',
+      condition: { dialect: 'cel', source: 'amount > 10' },
+    };
+    // …including the optional authorship `meta` that a "corrected" restatement
+    // would be the next thing to drop.
+    const annotated: SimEdge = {
+      source: 'd',
+      target: 'hi',
+      condition: { dialect: 'cel', source: 'amount > 10', meta: { generatedBy: 'agent' } },
+    };
+    expect(guarded.condition).toBeTruthy();
+    expect(annotated.condition).toBeTruthy();
+  });
+});

packages/app-shell/src/views/metadata-admin/previews/simulator/__tests__/flow-simulator.test.ts

@@ -1,6 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { describe, it, expect } from 'vitest';
+import { FlowEdgeSchema } from '@objectstack/spec/automation';
 import { FlowSimulator } from '../flow-simulator';
 import { validateFlowDraft, findCycle } from '../flow-sim-validate';
 import type { SimEdge, SimNode } from '../flow-sim-types';
@@ -12,6 +13,23 @@ const run = (nodes: SimNode[], edges: SimEdge[], seed = {}, mocks = {}) => {
   return sim;
 };
 
+/**
+ * An edge as the PLATFORM persists it, not as a test hand-writes it: authoring
+ * input goes through `FlowEdgeSchema`, whose `ExpressionInput` pipe rewrites a
+ * bare guard string into the `{ dialect: 'cel', source }` envelope. So the
+ * envelope is not an exotic spelling the simulator may decline to support — it
+ * is THE canonical persisted form, and a fixture that spells it by hand could
+ * drift from what the spec actually emits (objectui#3216).
+ *
+ * The `SimEdge` return annotation states the matching type-level claim, but do
+ * not read it as a proof: `tsc` never sees this file (the package tsconfig
+ * excludes `**\/*.test.ts`, and app-shell's test tree is still TEST_DEBT), and
+ * vitest erases types. The enforced version lives next door in
+ * `flow-sim-edge.types.test.ts`, which IS compiled — `PersistedEdge extends
+ * SimEdge`.
+ */
+const specEdge = (e: Record<string, unknown>): SimEdge => FlowEdgeSchema.parse(e);
+
 describe('validateFlowDraft', () => {
   it('flags a missing entry node', () => {
     const v = validateFlowDraft(
@@ -464,3 +482,111 @@ describe('FlowSimulator', () => {
     expect(jStep.note).toMatch(/not modelled/i);
   });
 });
+
+/**
+ * objectui#3216 — the simulator reads an edge guard through `conditionText`,
+ * the one reader every consumer of that field goes through.
+ *
+ * Both readers here (`flow-simulator.ts`'s decision routing and
+ * `flow-sim-validate.ts`'s diagnostics) used to accept only a bare string and
+ * return `undefined` for anything else. That made the simulator report
+ * `Branch has no condition.` for — and skip — a branch the ENGINE evaluates
+ * normally, i.e. it silently simulated semantics that differ from the runtime,
+ * which is the one thing the simulator's own contract forbids. The spelling it
+ * could not read is the spelling the platform itself produces (see
+ * {@link specEdge}), so this was never "the simulator supports the shorthand
+ * only"; it was a missing branch in two hand-rolled copies of one read.
+ */
+describe('decision guards in the spec expression envelope (#3216)', () => {
+  const NODES: SimNode[] = [
+    { id: 's', type: 'start' },
+    { id: 'd', type: 'decision' },
+    { id: 'hi', type: 'end' },
+    { id: 'lo', type: 'end' },
+  ];
+  const START: SimEdge = { id: 'e_s', source: 's', target: 'd' };
+
+  /** The decision's recorded evaluation of its out-edge to `target`. */
+  const edgeEval = (sim: FlowSimulator, target: string) => {
+    const step = sim.state.steps.find((x) => x.nodeId === 'd');
+    const found = (step?.edges ?? []).find((x) => x.target === target);
+    if (!found) throw new Error(`decision "d" recorded no evaluation for its edge to "${target}"`);
+    return found;
+  };
+
+  it('the spec canonicalises a bare guard INTO the envelope (premise of this suite)', () => {
+    const edge = specEdge({ id: 'e_hi', source: 'd', target: 'hi', condition: 'amount > 10' });
+    expect(edge.condition).toEqual({ dialect: 'cel', source: 'amount > 10' });
+  });
+
+  it('selects the branch whose envelope guard is true (the issue repro)', () => {
+    const guarded = specEdge({ id: 'e_hi', source: 'd', target: 'hi', condition: 'amount > 10' });
+    const sim = run(NODES, [START, guarded, { id: 'e_lo', source: 'd', target: 'lo', isDefault: true }], { amount: 20 });
+
+    expect(sim.state.visitedNodeIds).toContain('hi');
+    expect(sim.state.visitedNodeIds).not.toContain('lo');
+    const dStep = sim.state.steps.find((x) => x.nodeId === 'd');
+    expect(dStep?.edges?.find((x) => x.selected)?.target).toBe('hi');
+    // The timeline shows the guard it evaluated, not "Branch has no condition."
+    const hiEval = edgeEval(sim, 'hi');
+    expect(hiEval.condition).toBe('amount > 10');
+    expect(hiEval.error).toBeUndefined();
+  });
+
+  it('EVALUATES an envelope guard rather than treating it as always-true', () => {
+    const guarded = specEdge({ id: 'e_hi', source: 'd', target: 'hi', condition: 'amount > 10' });
+    const sim = run(NODES, [START, guarded, { id: 'e_lo', source: 'd', target: 'lo', isDefault: true }], { amount: 5 });
+
+    expect(sim.state.visitedNodeIds).toContain('lo');
+    expect(sim.state.visitedNodeIds).not.toContain('hi');
+    const hiEval = edgeEval(sim, 'hi');
+    // False for the right REASON: the guard ran and was false…
+    expect(hiEval.result).toBe(false);
+    expect(hiEval.condition).toBe('amount > 10');
+    // …not because the reader could not see a condition at all.
+    expect(hiEval.error).toBeUndefined();
+  });
+
+  it('still says "no condition" for an envelope with nothing readable to evaluate', () => {
+    // Spec phase M9.2 will emit `ast`-only envelopes. There is no CEL source to
+    // run, and the simulator's rule is to say so rather than fake a result —
+    // the fix widened the reader, it did not make every object a condition.
+    const astOnly: SimEdge = { id: 'e_hi', source: 'd', target: 'hi', condition: { dialect: 'cel', ast: { op: 'gt' } } };
+    const sim = run(NODES, [START, astOnly, { id: 'e_lo', source: 'd', target: 'lo', isDefault: true }], { amount: 20 });
+
+    const hiEval = edgeEval(sim, 'hi');
+    expect(hiEval.error).toBe('Branch has no condition.');
+    expect(sim.state.visitedNodeIds).toContain('lo');
+  });
+
+  it('reports a dead-ending envelope guard as false, not as an absent condition', () => {
+    const guarded = specEdge({ id: 'e_hi', source: 'd', target: 'hi', condition: 'amount > 10' });
+    const sim = run(NODES, [START, guarded], { amount: 5 });
+
+    expect(sim.state.status).toBe('error');
+    const hiEval = edgeEval(sim, 'hi');
+    expect(hiEval.error).toBeUndefined();
+    expect(hiEval.condition).toBe('amount > 10');
+  });
+
+  // The same under-read, one layer up: `validateFlowDraft` warns when a decision
+  // guards EVERY out-edge and has no default (it may dead-end). Reading only the
+  // bare string made a decision whose guards are envelopes silently exempt.
+  it('warns about a missing default when every guard is an envelope', () => {
+    const v = validateFlowDraft(NODES, [
+      START,
+      specEdge({ id: 'e_hi', source: 'd', target: 'hi', condition: 'amount > 10' }),
+      specEdge({ id: 'e_lo', source: 'd', target: 'lo', condition: 'amount <= 10' }),
+    ]);
+    expect(v.warnings.some((w) => w.nodeId === 'd' && /no default branch/.test(w.message))).toBe(true);
+  });
+
+  it('does not warn when one branch is unguarded (that branch cannot dead-end)', () => {
+    const v = validateFlowDraft(NODES, [
+      START,
+      specEdge({ id: 'e_hi', source: 'd', target: 'hi', condition: 'amount > 10' }),
+      { id: 'e_lo', source: 'd', target: 'lo' },
+    ]);
+    expect(v.warnings.some((w) => w.nodeId === 'd' && /no default branch/.test(w.message))).toBe(false);
+  });
+});

packages/app-shell/src/views/metadata-admin/previews/simulator/flow-sim-types.ts

@@ -14,6 +14,8 @@
  * "success".
  */
 
+import type { ExpressionInput } from '@objectstack/spec/shared';
+
 export interface SimNode {
   id: string;
   type: string;
@@ -27,7 +29,24 @@ export interface SimEdge {
   id?: string;
   source: string;
   target: string;
-  condition?: string | { source?: string };
+  /**
+   * Optional guard, in the spec's authoring shape: a bare CEL string, or the
+   * ADR-0089 envelope (`{ dialect, source }`) that `ExpressionInputSchema`
+   * normalizes every authored string INTO at parse time — so the envelope is
+   * the form a persisted flow actually carries, not an exotic alternative.
+   *
+   * Imported from the spec rather than restated. The local spelling used to be
+   * `string | { source?: string }`, and a restatement gets it wrong in BOTH
+   * directions at once: too WIDE, because it describes a `dialect`-less
+   * envelope that `FlowEdgeSchema` rejects outright; too NARROW, because
+   * excess-property checking then refuses the canonical envelope written as a
+   * literal (`dialect` "does not exist" on `{ source?: string }`). Correcting
+   * it by hand only moves the drift — the next spelling omits `ast` or `meta`.
+   * objectui#3202 removed the same restatement from `FlowDesignerEdge`; this
+   * was its last copy, and the reason the simulator could not read a guard the
+   * platform itself produces (objectui#3216).
+   */
+  condition?: ExpressionInput;
   isDefault?: boolean;
   label?: string;
   /**