Skip to content

Commit e6e4199

Browse files
fix(app-shell): an expression field shows the draft's server verdict at its own path (#26999)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8257mqSrsY5YAmowqeNZ7Vs)_ Fixes #26858 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:表达式字段(RLS 的 USING/CHECK、流程边条件)按自己的路径读取草稿 `_diagnostics` 的服务端裁决;该路径有错误时显示服务端消息,不再显示 "Valid CEL"。lint 的边条件 `expression-invalid` 发现补上 `flows[N].edges[M].condition` 路径。 - 为什么改:#26856 之后横幅已显示发布裁决,但字段仍打印自己的客户端结论,两者矛盾(裁定 p2)。 - 风险与回滚:只读路径相等匹配,不复制任何发布规则,不改任何规则的裁决或消息;回滚即 revert 本 PR。 - 先看:`ui/packages/app-shell/src/views/metadata-admin/draftVerdict.ts`、`CelPredicateField.tsx`、`packages/lint/src/validate-expressions.ts`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/serverVerdictAtPath-26858.test.tsx` 与 `pnpm --filter @objectstack/lint exec vitest run src/validate-expressions.edge-condition-path.test.ts`。 - 需要你做的:评审通过后合并;无需其他操作。 ## What changes - **lint**: `ExprIssue` gains an optional `path`. A top-level flow edge's `condition` finding carries `flows[N].edges[M].condition` beside its unchanged `where`; `authoring-rules` maps it as `path: i.path ?? i.where`. The served draft verdict therefore lands at `edges.M.condition` (via the existing `itemRelativeIssuePath`). Rule, verdict and message are unchanged; the runtime gate's differential fingerprint compares the same written flow on both sides. - **app-shell**: `DraftVerdictContext` plus `useServerErrorsAt(path)` in `draftVerdict.ts`, an equality-only lookup on the served path, never a prefix. - `CelPredicateField` takes `verdictPath`; when the served verdict has errors at it, the field shows the server message, sets `aria-invalid`, and withholds "Valid CEL". RLS clauses pass `rowLevelSecurity.N.using` / `.check`. - `FlowEdgeInspector` shows served errors at `edges.N.condition` under the condition. - Providers: `ResourceEditPage` (its served diagnostics), `PermissionMatrixEditPage` (its own draft read, refreshed after each save), and the Studio Automations page (its own flow draft read, refreshed after each save). ## Why Ruling p2 on #26858: after #26856 the expression field still printed its own client verdict where the draft's server verdict at that path said otherwise (an RLS predicate over a formula field, a flow edge condition). The field now shows the `_diagnostics` verdict at its own path whenever one exists. No publish rule is copied into the client. ## Acceptance notes Noted by the implementer, not filed (seat-applied body amendment): - **Class a candidate (read inference only, reach not measured at a public door):** Studio Automations (`AutomationsPillar` in `StudioDesignSurface.tsx`) shows no draft-level publish verdict in its banner or canvas Problems panel. #26856 wired that for the metadata-admin `ResourceEditPage` only; this PR adds only the field-level provider. - The field-permission `when` CEL field in `PermissionMatrixEditor` gets no `verdictPath`: no served rule was measured at such a path. - The `lint-flow-patterns` `flow-default-edge-with-condition` finding has no key path, so its served verdict cannot land on an edge field. ## Verification - New tests: lint 6 (edge path, null member, second flow, map-form no path, start-node no path, runtime gate path); UI 13 across `serverVerdictAtPath-26858`, `PermissionMatrixEditor.serverVerdict-26858`, `ResourceEditPage.serverVerdict-26858`, `AutomationsPillar.serverVerdict-26858`. They cover a refused RLS predicate (server message, no "Valid CEL"), a refused edge condition, the no-verdict control, the sibling-path no-leak case, and the lint finding's `edges.N.condition`. - Ablations (each restored, blob equal to HEAD): dropping the context read in `CelPredicateField` turns 4 field tests red; in `FlowEdgeInspector`, 3; deleting the lint path assignment, 4; mapping `i.where` only, 1. Controls stay green in each. - `@objectstack/lint` full suite 7515 passed plus typecheck; `@object-ui/app-shell` type-check; related app-shell suites 1129 passed; after merging `main`, the touched suites 199 passed. - `dispatch-gates --commands`: 63 families run, each exit 0; `--ran` reconciles 63 derived, 63 run, 0 NOT-MEASURED. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (spec untouched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: n/a. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [ ] 8. Title, body and comments are in English, except the dispatch-required Chinese maintainer brief above. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JQwxbYiz7aUf8wSAnnRyMx --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 07d18e6 commit e6e4199

16 files changed

Lines changed: 922 additions & 36 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/lint': patch
3+
---
4+
5+
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
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
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.
11+
12+
- `where`, the message and the verdict are unchanged: nothing is accepted or refused differently.
13+
- The edge is indexed in the list as authored (a `null` member before it does not renumber it).
14+
- A nested region's edge, and a flow in a map-form `flows` collection, keep `where` as their `path`, as before.
15+
- `ExprIssue` gains the optional `path`.

‎packages/lint/src/authoring-rules.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
791791
severity: i.severity ?? 'error',
792792
rule: EXPRESSION_INVALID,
793793
where: i.where,
794-
path: i.where,
794+
// [#26858] The slot's key path where the pass spells one (a flow edge's
795+
// `condition`), so a draft's `_diagnostics` can address the field.
796+
path: i.path ?? i.where,
795797
// [#22161] The authored source is a QUOTE of what the author wrote, not
796798
// a fix, so it rides the verdict — `— source: \`…\`` is the spelling
797799
// the flow engine's runtime refusals already use — and reaches the CLI
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #26858 — a flow edge's `condition` finding carries the slot's key path.
5+
*
6+
* `expression-invalid` is what publish refuses a flow edge's CEL with, and its
7+
* finding was located by its `where` prose alone (`flow 'f' · edge 'e2'
8+
* (decide→end) condition`). A draft's `_diagnostics` is item-relative and
9+
* dotted (`edges.1.condition`), so Studio's edge condition field could not ask
10+
* whether the refusal was its own. The finding now carries the key path in the
11+
* spelling the other rules use (`flows[0].edges[1].condition`) beside the
12+
* unchanged `where` and message.
13+
*/
14+
15+
import { describe, expect, it } from 'vitest';
16+
17+
import { runRuntimeAuthoringRules } from './runtime-gate.js';
18+
import { validateStackExpressions } from './validate-expressions.js';
19+
20+
type AnyRec = Record<string, unknown>;
21+
22+
const BROKEN = 'record.name ==';
23+
24+
function flow(edges: unknown[], extra: AnyRec = {}): AnyRec {
25+
return {
26+
name: 'edge_cel',
27+
label: 'Edge CEL',
28+
type: 'autolaunched',
29+
nodes: [
30+
{ id: 'start', type: 'start', label: 'Start' },
31+
{ id: 'decide', type: 'decision', label: 'Decide' },
32+
{ id: 'end', type: 'end', label: 'End' },
33+
],
34+
edges,
35+
...extra,
36+
};
37+
}
38+
39+
const EDGES = [
40+
{ id: 'e1', source: 'start', target: 'decide' },
41+
{ id: 'e2', source: 'decide', target: 'end', type: 'conditional', condition: BROKEN },
42+
];
43+
44+
describe('expression-invalid on a flow edge condition carries a key path (#26858)', () => {
45+
it('names the edge by its index in the authored list, where and message unchanged', () => {
46+
const issues = validateStackExpressions({ flows: [flow(EDGES)] });
47+
const onEdge = issues.filter((i) => i.where.includes("edge 'e2'"));
48+
expect(onEdge.length).toBeGreaterThan(0);
49+
for (const i of onEdge) {
50+
expect(i.path).toBe('flows[0].edges[1].condition');
51+
expect(i.where).toBe("flow 'edge_cel' · edge 'e2' (decide→end) condition");
52+
}
53+
});
54+
55+
it('indexes the raw list: a null member before the edge does not renumber it', () => {
56+
const issues = validateStackExpressions({ flows: [flow([null, ...EDGES])] });
57+
const onEdge = issues.filter((i) => i.where.includes("edge 'e2'"));
58+
expect(onEdge.length).toBeGreaterThan(0);
59+
for (const i of onEdge) expect(i.path).toBe('flows[0].edges[2].condition');
60+
});
61+
62+
it('indexes the flow in the stack', () => {
63+
const other = { ...flow([{ id: 'x1', source: 'start', target: 'end' }]), name: 'clean' };
64+
const issues = validateStackExpressions({ flows: [other, flow(EDGES)] });
65+
const onEdge = issues.filter((i) => i.where.includes("edge 'e2'"));
66+
expect(onEdge.length).toBeGreaterThan(0);
67+
for (const i of onEdge) expect(i.path).toBe('flows[1].edges[1].condition');
68+
});
69+
70+
it('a map-form flows collection has no index to spell, so the finding keeps where alone', () => {
71+
const { name: _name, ...body } = flow(EDGES);
72+
const issues = validateStackExpressions({ flows: { edge_cel: body } });
73+
const onEdge = issues.filter((i) => i.where.includes("edge 'e2'"));
74+
expect(onEdge.length).toBeGreaterThan(0);
75+
for (const i of onEdge) expect(i.path).toBeUndefined();
76+
});
77+
78+
it('a finding off the edge condition gains no path', () => {
79+
const issues = validateStackExpressions({
80+
flows: [
81+
flow([{ id: 'e1', source: 'start', target: 'end' }], {
82+
nodes: [
83+
{ id: 'start', type: 'start', label: 'Start', config: { condition: BROKEN } },
84+
{ id: 'end', type: 'end', label: 'End' },
85+
],
86+
}),
87+
],
88+
});
89+
expect(issues.length).toBeGreaterThan(0);
90+
for (const i of issues) expect(i.path).toBeUndefined();
91+
});
92+
93+
it('reaches the publish gate: the refusal is located at the edge condition, message unchanged', () => {
94+
const result = runRuntimeAuthoringRules({ type: 'flow', item: flow(EDGES) });
95+
const refusals = result.errors.filter((f) => f.rule === 'expression-invalid');
96+
expect(refusals.length).toBeGreaterThan(0);
97+
for (const f of refusals) {
98+
expect(f.path).toMatch(/^flows(\[0\]|\.edge_cel)\.edges\[1\]\.condition$/);
99+
expect(f.where).toBe("flow 'edge_cel' · edge 'e2' (decide→end) condition");
100+
}
101+
});
102+
});

‎packages/lint/src/validate-expressions.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,13 @@ export interface ExprIssue {
161161
* condition, which might be a flow variable). Absent ⇒ treat as `error`.
162162
*/
163163
severity?: 'error' | 'warning';
164+
/**
165+
* [#26858] The judged slot's key path in the stack, in the spelling the other
166+
* rules' findings carry (`flows[0].edges[1].condition`), beside the human
167+
* `where`. Set only where the slot has one: a top-level flow edge's
168+
* `condition`. Absent, the finding is located by `where` alone.
169+
*/
170+
path?: string;
164171
}
165172

166173
type AnyRec = Record<string, unknown>;
@@ -2511,6 +2518,10 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
25112518
objectWrite || runtimeGateWrite ? new Map() : flowCelEntrances(stack, fieldIndex);
25122519
for (const flow of objectWrite ? [] : recordsOf(stack.flows)) {
25132520
const flowName = typeof flow.name === 'string' ? flow.name : '(unnamed flow)';
2521+
// [#26858] The flow's index in the authored list, for a finding's key path
2522+
// (`recordsOf` hands an array's own members back). A map-form `flows` has
2523+
// no index to spell, so its findings keep `where` alone.
2524+
const flowIndex = Array.isArray(stack.flows) ? stack.flows.indexOf(flow) : -1;
25142525
// `Array.isArray` proves the LIST, never its MEMBERS — the sentence #15742
25152526
// removed from one reader below in this same file. A YAML `nodes:` item
25162527
// left empty deserialises to `null`, and `nodes.find(n => n.type === …)` on
@@ -2897,12 +2908,23 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
28972908
// and not a check.
28982909
for (const edge of recordsOf(graph.edges)) {
28992910
const edgeCondWhere = `${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`;
2911+
const first = issues.length;
29002912
if (!checkStructuralCondition(edgeCondWhere, edge.condition).refused) {
29012913
const from = issues.length;
29022914
check(edgeCondWhere, edge.condition, objectName);
29032915
warnShadowedFieldReads(edgeCondWhere, edge.condition);
29042916
refuseUnboundRoots(edgeCondWhere, edge.condition, from, 'predicate');
29052917
}
2918+
// [#26858] A top-level edge's findings also carry the slot's key path,
2919+
// indexed over the list the author wrote (the coerced one drops `null`
2920+
// members and would renumber). A nested region's edge keeps `where`.
2921+
const edgeIndex = flowIndex >= 0 && graph.path.length === 0 && Array.isArray(flow.edges)
2922+
? flow.edges.indexOf(edge)
2923+
: -1;
2924+
if (edgeIndex >= 0) {
2925+
const path = `flows[${flowIndex}].edges[${edgeIndex}].condition`;
2926+
for (let k = first; k < issues.length; k++) issues[k] = { ...issues[k]!, path };
2927+
}
29062928
}
29072929
// No `checkNullGuards` on node/edge conditions — and NOT for the reason
29082930
// #4811 first recorded (#4811 re-measured it). The stated blocker was the

‎ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {
5454
type CelSuggestionKind,
5555
type CelValueType,
5656
} from './celAuthoring.js';
57+
import { useServerErrorsAt } from './draftVerdict.js';
5758

5859
const LINT_DEBOUNCE_MS = 250;
5960

@@ -118,6 +119,15 @@ export interface CelPredicateFieldProps {
118119
* has no such second verdict (the permission matrix included).
119120
*/
120121
scopeIssue?: boolean;
122+
/**
123+
* objectstack#26858 — the item-relative path this editor writes in the body
124+
* the page holds (`rowLevelSecurity.0.using`). When the page's served verdict
125+
* (`DraftVerdictContext`) has an error at exactly that path — what publish
126+
* will refuse — the editor shows the server's message and withholds "Valid
127+
* CEL". The lint, its findings and `onLintChange` are unaffected, as with
128+
* `scopeIssue`. Omitted: no server verdict is read.
129+
*/
130+
verdictPath?: string;
121131
/** Reports the current lint issues up so the editor can gate Save on errors. */
122132
onLintChange?: (issues: CelLintIssue[]) => void;
123133
/**
@@ -158,6 +168,7 @@ export function CelPredicateField({
158168
slot,
159169
role,
160170
scopeIssue,
171+
verdictPath,
161172
onLintChange,
162173
onInferredTypeChange,
163174
t,
@@ -354,9 +365,13 @@ export function CelPredicateField({
354365

355366
const errors = issues.filter((i) => i.severity === 'error');
356367
const warnings = issues.filter((i) => i.severity === 'warning');
357-
// "Valid CEL" only when nothing — the lint, or the host's scope verdict —
358-
// says otherwise (objectui#11789).
359-
const clean = linted && !!value.trim() && issues.length === 0 && !scopeIssue;
368+
// objectstack#26858 — the server's errors at this editor's path, each one
369+
// the lint does not already say in the same words.
370+
const atPath = useServerErrorsAt(verdictPath);
371+
const serverErrors = atPath.filter((e) => !issues.some((iss) => issueText(iss, t) === e.message));
372+
// "Valid CEL" only when nothing — the lint, the host's scope verdict
373+
// (objectui#11789) or the server's verdict at this path — says otherwise.
374+
const clean = linted && !!value.trim() && issues.length === 0 && !scopeIssue && atPath.length === 0;
360375
// Result-type affordance (role="value"): shown once the expression parses,
361376
// even alongside warnings — the type is what dataset measure eligibility
362377
// keys off, so the author should see it whenever it is known.
@@ -384,7 +399,7 @@ export function CelPredicateField({
384399
aria-expanded={open}
385400
aria-autocomplete="list"
386401
aria-controls={open ? listId : undefined}
387-
aria-invalid={errors.length > 0 || undefined}
402+
aria-invalid={errors.length > 0 || atPath.length > 0 || undefined}
388403
onChange={(e) => {
389404
onChange(e.target.value);
390405
refreshAc();
@@ -397,7 +412,7 @@ export function CelPredicateField({
397412
}}
398413
className={cn(
399414
'font-mono text-xs',
400-
errors.length > 0 && 'border-destructive focus-visible:ring-destructive/40',
415+
(errors.length > 0 || atPath.length > 0) && 'border-destructive focus-visible:ring-destructive/40',
401416
)}
402417
/>
403418
{open && suggestions.length > 0 && (
@@ -431,15 +446,21 @@ export function CelPredicateField({
431446
)}
432447
</div>
433448

434-
{/* Inline lint findings. */}
435-
{(errors.length > 0 || warnings.length > 0) && (
449+
{/* Inline lint findings, then the server's at this path (as written). */}
450+
{(errors.length > 0 || warnings.length > 0 || serverErrors.length > 0) && (
436451
<ul className="mt-1 space-y-1" aria-live="polite">
437452
{errors.map((iss, i) => (
438453
<li key={`e${i}`} className="flex items-start gap-1.5 text-[11px] text-destructive">
439454
<AlertCircle className="mt-[1px] h-3 w-3 shrink-0" />
440455
<span>{issueText(iss, t)}</span>
441456
</li>
442457
))}
458+
{serverErrors.map((e, i) => (
459+
<li key={`s${i}`} className="flex items-start gap-1.5 text-[11px] text-destructive" data-testid="cel-server-error">
460+
<AlertCircle className="mt-[1px] h-3 w-3 shrink-0" />
461+
<span>{e.message}</span>
462+
</li>
463+
))}
443464
{warnings.map((iss, i) => (
444465
<li key={`w${i}`} className="flex items-start gap-1.5 text-[11px] text-amber-600 dark:text-amber-500">
445466
<AlertTriangle className="mt-[1px] h-3 w-3 shrink-0" />

‎ui/packages/app-shell/src/views/metadata-admin/PermissionAdvancedFacets.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ export function PermissionAdvancedFacets({
745745
objectName={pol.object}
746746
fieldNames={fieldsFor(pol.object)}
747747
clause="using"
748+
verdictPath={`rowLevelSecurity.${i}.using`}
748749
onLintChange={(issues) => reportClause(`${i}:using`, issues)}
749750
onChange={(v) => {
750751
const next = [...policies];
@@ -761,6 +762,7 @@ export function PermissionAdvancedFacets({
761762
objectName={pol.object}
762763
fieldNames={fieldsFor(pol.object)}
763764
clause="check"
765+
verdictPath={`rowLevelSecurity.${i}.check`}
764766
onLintChange={(issues) => reportClause(`${i}:check`, issues)}
765767
onChange={(v) => {
766768
const next = [...policies];

‎ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.packageDoorDraftBase-26837.test.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,9 @@ describe('PermissionMatrixEditPage — the PACKAGE door saves onto the pending d
170170
// Out-of-scope rows come from the draft too: it is what this save replaces.
171171
expect(body.objects.b_order).toEqual(DRAFT.objects.b_order);
172172
expect(body.fields).toEqual(DRAFT.fields);
173-
// CONTROL: the save-time draft read ran (#1 is the load).
174-
expect(server.getDraftCalls).toBe(2);
173+
// CONTROL: the save-time draft read ran (#1 is the load). objectstack#26858 —
174+
// #3 is the read after the save that refreshes the draft's served verdict.
175+
await waitFor(() => expect(server.getDraftCalls).toBe(3));
175176
expect(server.savedOpts[0]).toMatchObject({ mode: 'draft', packageId: 'app.a' });
176177
expect(screen.queryByText(REFUSAL)).toBeNull();
177178
});

0 commit comments

Comments
 (0)