Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/tidy-jars-shave.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
---
'@objectstack/lint': minor
---

Warn when a bare identifier in a flow node/edge condition is shadowed by a declared flow variable

A flow `condition` is evaluated in a flattened scope, so a bare `status` normally
resolves to the trigger record's field and is the correct, canon-taught spelling.
`objectstack validate` deliberately never judged a bare identifier there, and it
still does not — with one exception it now names.

When the same name is BOTH a declared flow variable and a field on the bound
object, the two collide silently: a run seeds its declared variables first and
flattens the record's fields only where nothing is bound yet, so the variable
wins, the field is unreachable under its own name, and nothing anywhere reports
it. The author reads `status` and gets the variable. On this surface that is the
least visible failure there is — a flow condition that never fires produces no
record, no error and no log line.

`validateStackExpressions` now emits a `warning` (never an error) on exactly that
case, naming the mechanism and both repairs: `record.status` for the field, or
rename the variable. A bare name that is only a field, or only a variable, stays
silent as before.

The variable set is collected across every ADR-0031 region of the flow, since a
run holds one variable map: flow-level declarations, loop/map iterator and index
variables, the try/catch error variable, node output variables, assignment
targets in all three shapes the executor accepts (including a legacy assignment
node with no `assignments` wrapper, whose top-level config keys are the variable
names), and node ids, which are bare CEL roots at runtime.
278 changes: 278 additions & 0 deletions packages/lint/src/flow-variable-scope.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SCOPE_ROOTS } from '@objectstack/formula';
import {
FlowSchema,
FlowVariableSchema,
FlowNodeSchema,
LoopConfigSchema,
TryCatchConfigSchema,
} from '@objectstack/spec/automation';

import {
collectFlowVariableNames,
shadowedFieldReads,
shadowedFieldMessage,
VARIABLE_NAME_CONFIG_KEYS,
ASSIGNMENT_ENTRY_NAME_KEYS,
} from './flow-variable-scope.js';

/**
* Unit coverage for the #14089 collection surface. The end-to-end behaviour
* (which conditions warn, which stay silent) lives in
* `validate-expressions.test.ts`; what is pinned HERE is the collection
* surface's completeness, row by row, because the ruling's criterion is only as
* closed as this set is.
*/
describe('collectFlowVariableNames (#14089)', () => {
const graphOf = (...nodes: Array<Record<string, unknown>>) => [{ nodes }];

it('row 1 — the flow\'s own declared variables', () => {
const names = collectFlowVariableNames(
{ variables: [{ name: 'batch_size', type: 'number' }, { name: 'cursor', type: 'text' }] },
[],
);
expect([...names].sort()).toEqual(['batch_size', 'cursor']);
});

it('rows 2-6 — the four declared config keys whose VALUE is a name', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { iteratorVariable: 'item_row', indexVariable: 'i' } },
{ id: 'guard', type: 'try_catch', config: { errorVariable: 'caught' } },
{ id: 'fetch', type: 'query_records', config: { outputVariable: 'rows' } },
));
for (const expected of ['item_row', 'i', 'caught', 'rows']) expect(names.has(expected)).toBe(true);
});

it('row 7 — all THREE assignment shapes, including the wrapper-less one', () => {
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: { total: 1 } } },
)).has('total')).toBe(true);

expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { assignments: [{ variable: 'total', value: 1 }] } },
)).has('total')).toBe(true);

// Shape 3 — no wrapper at all. `logic-nodes.ts`'s `else` branch reads the
// config's own keys, and this is the row a hand-written collector misses.
expect(collectFlowVariableNames({}, graphOf(
{ id: 'a', type: 'assignment', config: { total: 1, label: 'x' } },
)).has('total')).toBe(true);
});

it('row 7 shape 2 — the `name` and `key` spellings the executor also accepts', () => {
const names = collectFlowVariableNames({}, graphOf({
id: 'a', type: 'assignment',
config: { assignments: [{ name: 'by_name', value: 1 }, { key: 'by_key', value: 2 }] },
}));
expect([...names].sort()).toEqual(['a', 'by_key', 'by_name']);
});

it('row 7 is gated on the node TYPE — a non-assignment node donates no config keys', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'fetch', type: 'http', config: { url: 'https://example.test', method: 'GET' } },
));
// Only the node id (row 8). `url` / `method` are config, not variables —
// reading them would manufacture a warning on any object with a `url` field.
expect([...names]).toEqual(['fetch']);
});

it('row 8 — a node id is collected, because it is a bare CEL root at runtime', () => {
const names = collectFlowVariableNames({}, graphOf({ id: 'lookup_owner', type: 'query_records' }));
expect(names.has('lookup_owner')).toBe(true);
});

it('is FLOW-scoped: every graph contributes to one flat set', () => {
// `collectFlowGraphs` yields each ADR-0031 region as its own graph, and
// `seedRunVariables` builds ONE map per run — so a name declared inside a
// region is in scope for the whole flow, not just that region.
const names = collectFlowVariableNames({}, [
{ nodes: [{ id: 'start', type: 'start' }] },
{ nodes: [{ id: 'inner', type: 'assignment', config: { region_local: 1 } }] },
]);
expect(names.has('region_local')).toBe(true);
});

it('tolerates the shapes an unparsed source can carry', () => {
expect([...collectFlowVariableNames({}, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: 'nonsense' }, [])]).toEqual([]);
expect([...collectFlowVariableNames({ variables: [null, 7, { type: 'text' }] }, [])]).toEqual([]);
expect([...collectFlowVariableNames({}, graphOf({ id: 'n', config: 'nonsense' }))]).toEqual(['n']);
});

/**
* The alias `control-flow.zod.ts` REJECTS by name. Reading it here would be
* consumer-side tolerance of a shape the schema refuses (Prime Directive #12)
* — and it cannot arrive on the parsed path this rule runs on anyway.
*/
it('does not read the rejected `itemVariable` alias', () => {
const names = collectFlowVariableNames({}, graphOf(
{ id: 'sweep', type: 'loop', config: { itemVariable: 'should_not_be_collected' } },
));
expect(names.has('should_not_be_collected')).toBe(false);
});
});

describe('shadowedFieldReads (#14089)', () => {
const vars = (...names: string[]) => new Set(names);

it('reports a bare name that is BOTH a variable and a field', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('status'), ['status', 'amount']))
.toEqual(['status']);
});

it('is silent when the name is a field only', () => {
expect(shadowedFieldReads('status == "dispatched"', vars('other'), ['status'])).toEqual([]);
});

it('is silent when the name is a variable only', () => {
expect(shadowedFieldReads('batch_size > 0', vars('batch_size'), ['status'])).toEqual([]);
});

it('is silent on the dotted spelling it prescribes', () => {
expect(shadowedFieldReads('record.status == "x"', vars('status'), ['status'])).toEqual([]);
});

it('reports every shadowed root in one predicate, in discovery order', () => {
const found = shadowedFieldReads('status == "x" && amount > 1', vars('status', 'amount'), ['status', 'amount']);
expect(found.sort()).toEqual(['amount', 'status']);
});

/**
* The reason `firstUndeclaredReference` is the oracle and
* `collectCelRootIdentifiers` is not (maintainer's implementation input, item
* 6). A comprehension macro binds its own variable; an AST root scan reports
* that binder as a root, so a macro variable sharing a field's name would be
* flagged for a collision that cannot exist. The declaredness oracle acts only
* on cel-js's own `Unknown variable` fault, so it never sees the binder.
*/
it('does not flag a comprehension-macro variable that shares a field name', () => {
expect(shadowedFieldReads(
'record.lines.exists(status, status.ok)',
vars('status'),
['status'],
)).toEqual([]);
});

it('does not flag a function name that shares a field name', () => {
expect(shadowedFieldReads('size(record.lines) > 0', vars('size'), ['size'])).toEqual([]);
});

it('costs nothing when the two authored sets do not intersect', () => {
expect(shadowedFieldReads('anything at all', vars('a'), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', new Set<string>(), ['b'])).toEqual([]);
expect(shadowedFieldReads('anything at all', vars('a'), [])).toEqual([]);
});

it('is empty on a source that does not parse — the syntax pass owns that defect', () => {
expect(shadowedFieldReads('status == ', vars('status'), ['status'])).toEqual([]);
});

/**
* ⚠️ The oracle's known, DELIBERATE blind spot, pinned so it is a recorded
* property rather than a surprise: `SCOPE_ROOTS` are declared in the strict
* environment, so a flow variable named after one of them is never reported as
* a bare root. That is an UNDER-report — the safe direction for a warning —
* and closing it means consulting the AST, which re-opens the macro-variable
* false positive above. The pin reads the real baseline rather than a copied
* word, so a future `SCOPE_ROOTS` member keeps this honest.
*/
it('under-reports a variable named after a SCOPE_ROOTS member (documented blind spot)', () => {
const root = SCOPE_ROOTS[0];
expect(SCOPE_ROOTS.length).toBeGreaterThan(0);
expect(shadowedFieldReads(`${root} == "x"`, vars(root), [root])).toEqual([]);
});
});

describe('shadowedFieldMessage (#14089)', () => {
it('names the mechanism and both repairs', () => {
const message = shadowedFieldMessage('status', 'duly_assignment');
expect(message).toContain('`status`');
expect(message).toContain('`duly_assignment`');
expect(message).toContain('record.status');
expect(message).toMatch(/rename the variable/);
});
});

/**
* ── The declared-key guard for this module (#5017's pattern, #14089's surface) ──
*
* `validate-expressions.test.ts` pins that every key its rule reads off a
* metadata receiver is one `@objectstack/spec` declares. This module reads
* metadata too, so it carries the same guard rather than escaping it by living
* in a different file: the collection surface is exactly where an undeclared
* key would go unnoticed, since a key nobody declares simply collects nothing
* and the diagnostic stays silent — a green gate over a surface nothing read.
*
* It is asserted against the module's EXPORTED constants rather than a scan of
* its source text. That is the stronger of the two: a source scan pins the
* spelling someone typed, while these pin the values the collection walk
* actually indexes with — and it needs no private comment-stripper, the class
* `check:comment-mask-adoption` exists to keep out of this tree.
*/

/**
* Declared keys of a schema, unwrapping the optional / lazy layers these
* schemas are built with. Mirrors `validate-expressions.test.ts`'s helper of the
* same name; `lazySchema` proxies a FUNCTION target, so the `typeof` guard has
* to admit both or every lazily-built schema answers "declares nothing" and the
* guard goes vacuous.
*/
function shapeKeysOf(schema: unknown, depth = 0): string[] {
const s = schema as { shape?: Record<string, unknown>; _def?: Record<string, unknown>; unwrap?: () => unknown };
if (!s || (typeof s !== 'object' && typeof s !== 'function') || depth > 12) return [];
if (s.shape) return Object.keys(s.shape);
const d = (s._def ?? {}) as Record<string, unknown>;
const getter = d.getter as (() => unknown) | undefined;
for (const next of [d.innerType, d.element, d.valueType, getter?.(), d.in, d.out]) {
const r = shapeKeysOf(next, depth + 1);
if (r.length) return r;
}
if (typeof s.unwrap === 'function') return shapeKeysOf(s.unwrap(), depth + 1);
return [];
}

describe('flow-variable-scope reads only keys the spec declares (meta-test)', () => {
it('the flow-level keys it reads are declared by `FlowSchema` / `FlowVariableSchema`', () => {
const flowKeys = shapeKeysOf(FlowSchema);
expect(flowKeys.length, 'FlowSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(flowKeys).toContain('variables');

const variableKeys = shapeKeysOf(FlowVariableSchema);
expect(variableKeys.length, 'FlowVariableSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
expect(variableKeys).toContain('name');
});

it('the node keys it reads are declared by `FlowNodeSchema`', () => {
const nodeKeys = shapeKeysOf(FlowNodeSchema);
expect(nodeKeys.length, 'FlowNodeSchema resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['id', 'type', 'config']) expect(nodeKeys).toContain(key);
});

it('the config-key list is exactly the four declared name-valued keys', () => {
expect([...VARIABLE_NAME_CONFIG_KEYS])
.toEqual(['iteratorVariable', 'indexVariable', 'errorVariable', 'outputVariable']);
// ⛔ The alias `control-flow.zod.ts` rejects BY NAME must not be here — reading
// it would be consumer-side tolerance of a shape the schema refuses (Prime
// Directive #12), and it cannot arrive on the parsed path this rule runs on.
expect([...VARIABLE_NAME_CONFIG_KEYS]).not.toContain('itemVariable');
// Every one of them is a key some node-config schema really declares — a
// list of keys nothing declares would collect nothing, silently.
const declaredAnywhere = new Set([
...shapeKeysOf(LoopConfigSchema),
...shapeKeysOf(TryCatchConfigSchema),
]);
expect(declaredAnywhere.size, 'the node-config schemas resolved to no keys — the guard would be vacuous').toBeGreaterThan(0);
for (const key of ['iteratorVariable', 'indexVariable']) expect(declaredAnywhere).toContain(key);
expect(declaredAnywhere).toContain('errorVariable');
});

it('the assignment entry-name keys are the three the executor reads, in its order', () => {
// The node TYPE itself is module-private (see the comment on it): a
// slug-shaped `export const` in this package is read as a rule id that a
// published barrel must carry. Its gate is pinned by BEHAVIOUR above —
// `row 7 is gated on the node TYPE` — which is the property that matters.
expect([...ASSIGNMENT_ENTRY_NAME_KEYS]).toEqual(['variable', 'name', 'key']);
});
});
Loading
Loading