fix(mcp): validate_expression takes the flow's declared variables at a flow condition

.changeset/33176-validate-expression-flow-variables.md

@@ -0,0 +1,15 @@
+---
+'@objectstack/mcp': patch
+'@objectstack/lint': patch
+---
+
+fix(mcp): `validate_expression` takes the flow's declared `variables` at a `flow_condition`, so a bare name binds as the flow binds it and the shadowed-read warning comes back
+
+Clause-②: yes (widening) — the MCP `validate_expression` tool accepts a new optional input, `variables`, at `site: 'flow_condition'`, a call its closed input refused before; without it every answer is unchanged.
+Metadata-protocol: none
+
+A flow run binds its declared variables before it flattens the trigger record's fields, so a bare name a variable shares with a field reads the variable, and the runtime flow publish door warns about it (a bare read of a name that is both a declared flow variable and a field). `validate_expression` was never handed the flow's variables: it typed such a bare name as the field, and it could not give that warning.
+
+- `variables` (`site: 'flow_condition'` only): the flow's declared `variables`, in the flow's own shape (`FlowVariableSchema`: a `name` and a `type` at least), so a flow's `variables` array can be handed over as it stands; a malformed one is refused with the spec's own sentence. Given them, a bare read of a declared variable is not typed as the field (the shared validator's `flowVariables`, as the gate hands it on the same flow), and a variable named like a field of `objectName` read bare gets the gate's own shadowed-read warning. `record.FIELD` still reads the field and is judged as before. On another site the input is refused, like `startCondition`.
+- Without `variables`, the answer is byte for byte what it was, and the tool description says the shadowed-read warning needs them.
+- `@objectstack/lint`: `judgeAuthoredSiteExpression` takes an optional `variables` (a new optional key on the exported `AuthoredSiteExpression` type), which its one-flow probe declares. The gate's verdicts are unchanged.

packages/lint/src/authored-site-expression.test.ts

@@ -452,6 +452,8 @@ type FlowFixture = {
   expression: string;
   severity: 'error' | 'warning';
   names: string;
+  /** [#33176] The flow's declared variables, handed to the entry as the caller hands them and declared on the hand-written flow. */
+  variables?: Array<Record<string, unknown>>;
 };
 
 type FlowClassified =
@@ -502,11 +504,17 @@ const FLOW_CALLS: Record<string, FlowClassified> = {
     fixture: { objects: [FLOW_OBJ], expression: 'foo.bar == 1' },
     buildDoor: {},
   },
+  // [#33176] The flow's declared variables, which the caller hands the entry
+  // and the probe declares; without them it cannot fire (pinned below).
   warnShadowedFieldReads: {
-    kind: 'pass-silent-here',
-    why: "needs the flow's declared variables, which the tool is not handed; the probe declares none",
-    fixture: { objects: [FLOW_OBJ], expression: "title == 'x'" },
-    buildDoor: { variables: [{ name: 'title', type: 'text' }] },
+    kind: 'pass',
+    fixture: {
+      objects: [FLOW_OBJ],
+      expression: "title == 'x'",
+      severity: 'warning',
+      names: '`title`',
+      variables: [{ name: 'title', type: 'text' }],
+    },
   },
   refuseStoredMetadataBodyHop: {
     kind: 'pass-silent-here',
@@ -597,7 +605,7 @@ function flowGateOn(
 function flowEntryOn(
   objects: Array<Record<string, unknown>>,
   expression: string,
-  opts: { triggerKind?: AuthoredFlowTrigger; startCondition?: string } = {},
+  opts: { triggerKind?: AuthoredFlowTrigger; startCondition?: string } & HandFlowOverrides = {},
 ) {
   return judgeAuthoredSiteExpression({ site: 'flow_condition', objectName: 'obj', expression, objects, ...opts })
     .map((i) => [i.severity ?? 'error', i.message]);
@@ -617,12 +625,13 @@ describe('judgeAuthoredSiteExpression at a flow condition — every gate pass is
 
   for (const [name, classified] of Object.entries(FLOW_CALLS)) {
     if (classified.kind === 'pass') {
-      const { objects, expression, severity, names } = classified.fixture;
+      const { objects, expression, severity, names, variables } = classified.fixture;
+      const declared = variables ? { variables } : {};
       for (const slot of ['start', 'decision', 'edge'] as const) {
         it(`${name} refuses through the entry as the gate does at a flow's ${slot} condition`, () => {
-          const gate = flowGateOn(objects, expression, { slot });
+          const gate = flowGateOn(objects, expression, { slot, ...declared });
           expect(gate.length, 'the fixture trips the gate').toBeGreaterThan(0);
-          const entry = flowEntryOn(objects, expression);
+          const entry = flowEntryOn(objects, expression, declared);
           expect(entry).toEqual(gate);
           expect(entry.map(([level]) => level)).toEqual(gate.map(() => severity));
           expect(entry.some(([, message]) => message!.includes(names))).toBe(true);
@@ -715,6 +724,58 @@ describe('judgeAuthoredSiteExpression at a flow condition — every gate pass is
   });
 });
 
+// ── [#33176] The flow's declared variables, at a flow condition ───────────
+//
+// Handed the flow's `variables`, the probe declares them, and the gate's
+// shadowed-read warning (`warnShadowedFieldReads`, a `pass` in the table
+// above with those variables) comes back through the entry. Handed none, the
+// probe is the flow it was before the key, and the warning cannot fire.
+
+describe("judgeAuthoredSiteExpression with the flow's declared variables (#33176)", () => {
+  const SHADOWING = [{ name: 'title', type: 'text' }];
+
+  it('declares the variables it is handed, as handed, and no `variables` key without them', () => {
+    const variables = [{ name: 'title', type: 'text', isInput: true }, { name: 'limit', type: 'number' }];
+    expect(authoredFlowProbe({ objectName: 'obj', expression: 'true', variables }).variables).toEqual(variables);
+    expect(Object.keys(authoredFlowProbe({ objectName: 'obj', expression: 'true' }))).not.toContain('variables');
+  });
+
+  it('CONTROL — without the variables the shadowed read answers nothing, at the gate and through the entry', () => {
+    // A flow that declares no variable gets no warning at the runtime door…
+    expect(flowGateOn([FLOW_OBJ], "title == 'x'")).toEqual([]);
+    // …and the probe, handed none, is that flow.
+    expect(flowEntryOn([FLOW_OBJ], "title == 'x'")).toEqual([]);
+  });
+
+  it('a variable named like no field of the object gets no warning', () => {
+    const variables = [{ name: 'threshold', type: 'number' }];
+    for (const expression of ['threshold > 3', "record.title == 'x' && threshold > 3"]) {
+      expect(flowGateOn([FLOW_OBJ], expression, { variables }), expression).toEqual([]);
+      expect(flowEntryOn([FLOW_OBJ], expression, { variables }), expression).toEqual([]);
+    }
+  });
+
+  it('warns on the bare read alone: `record.FIELD` reads the field, whatever the variables', () => {
+    expect(flowGateOn([FLOW_OBJ], "record.title == 'x'", { variables: SHADOWING })).toEqual([]);
+    expect(flowEntryOn([FLOW_OBJ], "record.title == 'x'", { variables: SHADOWING })).toEqual([]);
+    const both = flowGateOn([FLOW_OBJ], 'record.title == title', { variables: SHADOWING });
+    expect(both.map(([level]) => level)).toEqual(['warning']);
+    expect(flowEntryOn([FLOW_OBJ], 'record.title == title', { variables: SHADOWING })).toEqual(both);
+  });
+
+  it('answers as the gate on every trigger kind and after a start condition', () => {
+    for (const kind of ['record_change', 'time_relative', 'schedule', 'api', 'none'] as const) {
+      const gate = flowGateOn([FLOW_OBJ], "title == 'x'", { kind, variables: SHADOWING });
+      expect(gate.map(([level]) => level), kind).toEqual(['warning']);
+      expect(flowEntryOn([FLOW_OBJ], "title == 'x'", { triggerKind: kind, variables: SHADOWING }), kind).toEqual(gate);
+    }
+    const startCondition = 'record.days != null';
+    const gate = flowGateOn([FLOW_OBJ], "title == 'x'", { startCondition, variables: SHADOWING });
+    expect(gate.map(([level]) => level)).toEqual(['warning']);
+    expect(flowEntryOn([FLOW_OBJ], "title == 'x'", { startCondition, variables: SHADOWING })).toEqual(gate);
+  });
+});
+
 // ── [#33147] The field being authored, at the formula site ───────────────
 //
 // Named as `field`, a field the definition declares keeps its definition with

packages/lint/src/authored-site-expression.ts

@@ -33,12 +33,15 @@
  * door gives the null guard on a `record.` / `previous.` read and the
  * unprovisioned-anchor warning. It never judges a bare name's null guard or a
  * root the run does not bind: those are `os validate`'s whole-flow judgments,
- * over the flow's variables, node ids and entrances. The door does warn on a
- * bare read of a name a flow variable shares with a field
- * (`warnShadowedFieldReads`), but that needs the flow's declared variables,
- * and the caller does not hand this entry any, so the probe declares none and
- * that warning never fires here. So `[]` at a flow condition is that door's
- * verdict, short of the shadowed-read warning, and not `os validate`'s.
+ * over the flow's variables, node ids and entrances. So `[]` at a flow
+ * condition is that door's verdict, and not `os validate`'s.
+ *
+ * [#33176] The door also warns on a bare read of a name a declared flow
+ * variable shares with a field of the object (`warnShadowedFieldReads`): the
+ * variable wins at run time. That reads the flow's declared `variables`, so
+ * the probe declares the ones the caller hands it, as the flow declares them,
+ * and the gate's own warning comes back. Handed none, the probe declares none
+ * and that warning cannot fire, exactly as before this key.
  */
 
 import { analyzeRelationshipTraversals } from '@objectstack/formula';
@@ -96,6 +99,15 @@ export interface AuthoredSiteExpression {
    * start condition itself, or follows none (the gate answers both alike).
    */
   startCondition?: string;
+  /**
+   * [#33176] site `flow_condition` only: the flow's declared `variables`, in
+   * the shape `FlowSchema.variables` declares them (`FlowVariableSchema`: a
+   * `name` and a `type` at least). The probe declares them, so the gate binds
+   * a bare read of one to the variable and gives its shadowed-read warning
+   * where one shares a name with a field of `objectName`. Absent: the probe
+   * declares none, as before this key.
+   */
+  variables?: readonly AnyRec[];
   /**
    * The object definitions as the registry serves them: the subject's, and
    * those of the objects its references name that the expression reads
@@ -166,20 +178,26 @@ function probeTrigger(
  * [#33147] The flow the gate judges for one flow condition: a start node
  * bound to `objectName` with the trigger the caller names (and its start
  * condition, when given), and one edge to an end node carrying the
- * condition. Exported for the read-back pin; the entry is its one caller.
+ * condition; [#33176] it declares the flow's `variables` when the caller
+ * hands them. Exported for the read-back pin; the entry is its one caller.
  */
 export function authoredFlowProbe({
   objectName,
   expression,
   triggerKind = 'record_change',
   sweepObject,
   startCondition,
-}: Pick<AuthoredSiteExpression, 'objectName' | 'expression' | 'triggerKind' | 'sweepObject' | 'startCondition'>): AnyRec {
+  variables,
+}: Pick<
+  AuthoredSiteExpression,
+  'objectName' | 'expression' | 'triggerKind' | 'sweepObject' | 'startCondition' | 'variables'
+>): AnyRec {
   const trigger = probeTrigger(triggerKind, objectName, sweepObject);
   return {
     name: AUTHORED_FLOW_NAME,
     label: AUTHORED_FLOW_NAME,
     type: trigger.type,
+    ...(variables !== undefined ? { variables: [...variables] } : {}),
     nodes: [
       {
         id: 'start',
@@ -230,6 +248,7 @@ export function judgeAuthoredSiteExpression({
   triggerKind,
   sweepObject,
   startCondition,
+  variables,
   objects,
 }: AuthoredSiteExpression): ExprIssue[] {
   const authored = objects.map((o) => stripInjectedSystemColumns(o));
@@ -249,7 +268,7 @@ export function judgeAuthoredSiteExpression({
     // rules are not what is judged here (left out, as at a validation site).
     stack = {
       objects: [body, ...others],
-      flows: [authoredFlowProbe({ objectName, expression, triggerKind, sweepObject, startCondition })],
+      flows: [authoredFlowProbe({ objectName, expression, triggerKind, sweepObject, startCondition, variables })],
     };
     runtimeWriteType = 'flow';
     // Every finding on the probe's one edge carries that edge's key path.