diff --git a/.changeset/26858-edge-condition-finding-path.md b/.changeset/26858-edge-condition-finding-path.md new file mode 100644 index 00000000000..17794ccf0fc --- /dev/null +++ b/.changeset/26858-edge-condition-finding-path.md @@ -0,0 +1,15 @@ +--- +'@objectstack/lint': patch +--- + +fix(lint): a flow edge condition's `expression-invalid` finding carries the slot's key path (`flows[0].edges[1].condition`) beside its `where` text + +Clause-②: no +Metadata-protocol: none + +An `expression-invalid` finding on a top-level flow edge's `condition` used to be located by its `where` prose alone (`flow 'f' · edge 'e2' (decide→end) condition`), which a reader keyed on paths could not address. Its `path` is now the key path in the spelling the other rules' findings use, so a draft's `_diagnostics` reports it item-relative (`edges.1.condition`) and Studio's edge condition field shows the refusal under itself. + +- `where`, the message and the verdict are unchanged: nothing is accepted or refused differently. +- The edge is indexed in the list as authored (a `null` member before it does not renumber it). +- A nested region's edge, and a flow in a map-form `flows` collection, keep `where` as their `path`, as before. +- `ExprIssue` gains the optional `path`. diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 95b28ab2b71..04c6d823e81 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -791,7 +791,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ severity: i.severity ?? 'error', rule: EXPRESSION_INVALID, where: i.where, - path: i.where, + // [#26858] The slot's key path where the pass spells one (a flow edge's + // `condition`), so a draft's `_diagnostics` can address the field. + path: i.path ?? i.where, // [#22161] The authored source is a QUOTE of what the author wrote, not // a fix, so it rides the verdict — `— source: \`…\`` is the spelling // the flow engine's runtime refusals already use — and reaches the CLI diff --git a/packages/lint/src/validate-expressions.edge-condition-path.test.ts b/packages/lint/src/validate-expressions.edge-condition-path.test.ts new file mode 100644 index 00000000000..c298bb51546 --- /dev/null +++ b/packages/lint/src/validate-expressions.edge-condition-path.test.ts @@ -0,0 +1,102 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #26858 — a flow edge's `condition` finding carries the slot's key path. + * + * `expression-invalid` is what publish refuses a flow edge's CEL with, and its + * finding was located by its `where` prose alone (`flow 'f' · edge 'e2' + * (decide→end) condition`). A draft's `_diagnostics` is item-relative and + * dotted (`edges.1.condition`), so Studio's edge condition field could not ask + * whether the refusal was its own. The finding now carries the key path in the + * spelling the other rules use (`flows[0].edges[1].condition`) beside the + * unchanged `where` and message. + */ + +import { describe, expect, it } from 'vitest'; + +import { runRuntimeAuthoringRules } from './runtime-gate.js'; +import { validateStackExpressions } from './validate-expressions.js'; + +type AnyRec = Record; + +const BROKEN = 'record.name =='; + +function flow(edges: unknown[], extra: AnyRec = {}): AnyRec { + return { + name: 'edge_cel', + label: 'Edge CEL', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'decide', type: 'decision', label: 'Decide' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges, + ...extra, + }; +} + +const EDGES = [ + { id: 'e1', source: 'start', target: 'decide' }, + { id: 'e2', source: 'decide', target: 'end', type: 'conditional', condition: BROKEN }, +]; + +describe('expression-invalid on a flow edge condition carries a key path (#26858)', () => { + it('names the edge by its index in the authored list, where and message unchanged', () => { + const issues = validateStackExpressions({ flows: [flow(EDGES)] }); + const onEdge = issues.filter((i) => i.where.includes("edge 'e2'")); + expect(onEdge.length).toBeGreaterThan(0); + for (const i of onEdge) { + expect(i.path).toBe('flows[0].edges[1].condition'); + expect(i.where).toBe("flow 'edge_cel' · edge 'e2' (decide→end) condition"); + } + }); + + it('indexes the raw list: a null member before the edge does not renumber it', () => { + const issues = validateStackExpressions({ flows: [flow([null, ...EDGES])] }); + const onEdge = issues.filter((i) => i.where.includes("edge 'e2'")); + expect(onEdge.length).toBeGreaterThan(0); + for (const i of onEdge) expect(i.path).toBe('flows[0].edges[2].condition'); + }); + + it('indexes the flow in the stack', () => { + const other = { ...flow([{ id: 'x1', source: 'start', target: 'end' }]), name: 'clean' }; + const issues = validateStackExpressions({ flows: [other, flow(EDGES)] }); + const onEdge = issues.filter((i) => i.where.includes("edge 'e2'")); + expect(onEdge.length).toBeGreaterThan(0); + for (const i of onEdge) expect(i.path).toBe('flows[1].edges[1].condition'); + }); + + it('a map-form flows collection has no index to spell, so the finding keeps where alone', () => { + const { name: _name, ...body } = flow(EDGES); + const issues = validateStackExpressions({ flows: { edge_cel: body } }); + const onEdge = issues.filter((i) => i.where.includes("edge 'e2'")); + expect(onEdge.length).toBeGreaterThan(0); + for (const i of onEdge) expect(i.path).toBeUndefined(); + }); + + it('a finding off the edge condition gains no path', () => { + const issues = validateStackExpressions({ + flows: [ + flow([{ id: 'e1', source: 'start', target: 'end' }], { + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { condition: BROKEN } }, + { id: 'end', type: 'end', label: 'End' }, + ], + }), + ], + }); + expect(issues.length).toBeGreaterThan(0); + for (const i of issues) expect(i.path).toBeUndefined(); + }); + + it('reaches the publish gate: the refusal is located at the edge condition, message unchanged', () => { + const result = runRuntimeAuthoringRules({ type: 'flow', item: flow(EDGES) }); + const refusals = result.errors.filter((f) => f.rule === 'expression-invalid'); + expect(refusals.length).toBeGreaterThan(0); + for (const f of refusals) { + expect(f.path).toMatch(/^flows(\[0\]|\.edge_cel)\.edges\[1\]\.condition$/); + expect(f.where).toBe("flow 'edge_cel' · edge 'e2' (decide→end) condition"); + } + }); +}); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index e2f13e13626..0faec702003 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -161,6 +161,13 @@ export interface ExprIssue { * condition, which might be a flow variable). Absent ⇒ treat as `error`. */ severity?: 'error' | 'warning'; + /** + * [#26858] The judged slot's key path in the stack, in the spelling the other + * rules' findings carry (`flows[0].edges[1].condition`), beside the human + * `where`. Set only where the slot has one: a top-level flow edge's + * `condition`. Absent, the finding is located by `where` alone. + */ + path?: string; } type AnyRec = Record; @@ -2511,6 +2518,10 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression objectWrite || runtimeGateWrite ? new Map() : flowCelEntrances(stack, fieldIndex); for (const flow of objectWrite ? [] : recordsOf(stack.flows)) { const flowName = typeof flow.name === 'string' ? flow.name : '(unnamed flow)'; + // [#26858] The flow's index in the authored list, for a finding's key path + // (`recordsOf` hands an array's own members back). A map-form `flows` has + // no index to spell, so its findings keep `where` alone. + const flowIndex = Array.isArray(stack.flows) ? stack.flows.indexOf(flow) : -1; // `Array.isArray` proves the LIST, never its MEMBERS — the sentence #15742 // removed from one reader below in this same file. A YAML `nodes:` item // left empty deserialises to `null`, and `nodes.find(n => n.type === …)` on @@ -2897,12 +2908,23 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // and not a check. for (const edge of recordsOf(graph.edges)) { const edgeCondWhere = `${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`; + const first = issues.length; if (!checkStructuralCondition(edgeCondWhere, edge.condition).refused) { const from = issues.length; check(edgeCondWhere, edge.condition, objectName); warnShadowedFieldReads(edgeCondWhere, edge.condition); refuseUnboundRoots(edgeCondWhere, edge.condition, from, 'predicate'); } + // [#26858] A top-level edge's findings also carry the slot's key path, + // indexed over the list the author wrote (the coerced one drops `null` + // members and would renumber). A nested region's edge keeps `where`. + const edgeIndex = flowIndex >= 0 && graph.path.length === 0 && Array.isArray(flow.edges) + ? flow.edges.indexOf(edge) + : -1; + if (edgeIndex >= 0) { + const path = `flows[${flowIndex}].edges[${edgeIndex}].condition`; + for (let k = first; k < issues.length; k++) issues[k] = { ...issues[k]!, path }; + } } // No `checkNullGuards` on node/edge conditions — and NOT for the reason // #4811 first recorded (#4811 re-measured it). The stated blocker was the diff --git a/ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx b/ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx index a3963e09ad3..eae42795f22 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx @@ -54,6 +54,7 @@ import { type CelSuggestionKind, type CelValueType, } from './celAuthoring.js'; +import { useServerErrorsAt } from './draftVerdict.js'; const LINT_DEBOUNCE_MS = 250; @@ -118,6 +119,15 @@ export interface CelPredicateFieldProps { * has no such second verdict (the permission matrix included). */ scopeIssue?: boolean; + /** + * objectstack#26858 — the item-relative path this editor writes in the body + * the page holds (`rowLevelSecurity.0.using`). When the page's served verdict + * (`DraftVerdictContext`) has an error at exactly that path — what publish + * will refuse — the editor shows the server's message and withholds "Valid + * CEL". The lint, its findings and `onLintChange` are unaffected, as with + * `scopeIssue`. Omitted: no server verdict is read. + */ + verdictPath?: string; /** Reports the current lint issues up so the editor can gate Save on errors. */ onLintChange?: (issues: CelLintIssue[]) => void; /** @@ -158,6 +168,7 @@ export function CelPredicateField({ slot, role, scopeIssue, + verdictPath, onLintChange, onInferredTypeChange, t, @@ -354,9 +365,13 @@ export function CelPredicateField({ const errors = issues.filter((i) => i.severity === 'error'); const warnings = issues.filter((i) => i.severity === 'warning'); - // "Valid CEL" only when nothing — the lint, or the host's scope verdict — - // says otherwise (objectui#11789). - const clean = linted && !!value.trim() && issues.length === 0 && !scopeIssue; + // objectstack#26858 — the server's errors at this editor's path, each one + // the lint does not already say in the same words. + const atPath = useServerErrorsAt(verdictPath); + const serverErrors = atPath.filter((e) => !issues.some((iss) => issueText(iss, t) === e.message)); + // "Valid CEL" only when nothing — the lint, the host's scope verdict + // (objectui#11789) or the server's verdict at this path — says otherwise. + const clean = linted && !!value.trim() && issues.length === 0 && !scopeIssue && atPath.length === 0; // Result-type affordance (role="value"): shown once the expression parses, // even alongside warnings — the type is what dataset measure eligibility // keys off, so the author should see it whenever it is known. @@ -384,7 +399,7 @@ export function CelPredicateField({ aria-expanded={open} aria-autocomplete="list" aria-controls={open ? listId : undefined} - aria-invalid={errors.length > 0 || undefined} + aria-invalid={errors.length > 0 || atPath.length > 0 || undefined} onChange={(e) => { onChange(e.target.value); refreshAc(); @@ -397,7 +412,7 @@ export function CelPredicateField({ }} className={cn( 'font-mono text-xs', - errors.length > 0 && 'border-destructive focus-visible:ring-destructive/40', + (errors.length > 0 || atPath.length > 0) && 'border-destructive focus-visible:ring-destructive/40', )} /> {open && suggestions.length > 0 && ( @@ -431,8 +446,8 @@ export function CelPredicateField({ )} - {/* Inline lint findings. */} - {(errors.length > 0 || warnings.length > 0) && ( + {/* Inline lint findings, then the server's at this path (as written). */} + {(errors.length > 0 || warnings.length > 0 || serverErrors.length > 0) && (