Skip to content

Commit ddab8b9

Browse files
fix(studio): the flow Problems list reads the draft's publish verdict, and a blank expression says it needs one (#27826)
domain:ui#3 · os-bill · session_01Utp6CaJoinvGQSbHeDaWBe <!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wg4i9JiKjEjwtuVzd4gKy9hB)_ Fixes #27799 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio「自动化」里流程的「问题」面板接上了草稿读回时服务器给出的发布判定(之前只有步骤检查器在读它);值写成表达式却留空时,节点和问题面板都改说「这个值需要一个表达式」,不再显示信封的线上格式。 - 为什么改:节点按 spec 的值槽规则报错,发布门禁也按同一规则拒绝,但问题面板只看画布结构,于是一边报错一边说「没有问题——该流程结构有效」,作者照此发布会被拒。 - 风险与回滚:只动 ui(app-shell 私有包 + i18n 两条文案);问题面板现在会多出服务器判定里的条目(包括以前就存在、但 Studio 没显示的 schema 错误),属预期;回滚即 revert 本 PR。 - 先看:`StudioDesignSurface.tsx`(把 `flowVerdict` 交给画布的那一处)、`previews/flow-problems.ts`(服务器条目的定位与文案)、`inspectors/flow-value-envelope.ts`(`isBlankExpressionRefusal`)。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/AutomationsPillar.problemsAgree-27799.test.tsx`,预期 3 个用例全绿。 - 需要你做的:无,席位落地。 ## What changes Measured first, on `main` at `86aaff32`, with the Studio Automations page mounted against a server double that judges as the server does (the save door parses `FlowSchema`; the draft read folds in the publish gate's finding from the real `validateStackExpressions`). A record-change flow, an Update Records node, a value `id` switched to an expression and left blank: - the value cell showed the spec's refusal (`AssignmentValueSchema`, run live): "A value carrying a `dialect` key is read as an expression envelope … Write `{ dialect: 'cel', source: '…' }`"; - the draft save was accepted (the flow parse does not run the value-slot rule, by design: `flow-value-slot-template.ts`, "The doors that run it"), and the draft read's `_diagnostics` carried the publish gate's `expression-invalid` finding with the same message; - the Problems tab read "No problems — this flow is structurally valid.", because the page handed the served verdict to the inspectors (objectstack#26858) and never to the canvas: `FlowPreview` got no `diagnostics` there. So the blank expression is a defect (the publish refuses it), and the one judge is the spec's value-slot rule, which both the cell and the publish gate run. The fix wires Problems to that verdict; no client-side validator is added. - **Problems reads the served verdict.** The Automations page hands `flowVerdict` (the draft read's `_diagnostics`, re-read after each autosave) to the flow preview as `diagnostics`, through `verdictPreviewEntries` in `draftVerdict.ts`, with messages through `translateValidationMessage` as the metadata edit page does. - **The row names the node and the field.** The publish gate locates a finding on a node's config value in words (`flow 'touch_account' · node 'write' (update_record) update_record field value at config.fields.id`), not as an item path, so it cannot be placed on a node. A server row on no element now shows the server's locator as its target instead of the bare word "flow". - **Business words, from the spec's own verdict.** `isBlankExpressionRefusal` recognises the spec's refusal of a blank expression by asking the spec once what that refusal reads (`valueEnvelopeRefusal({ dialect: 'cel', source: '' })`), never by restating the rule. The value cell and the Problems row both say "This value needs an expression. Write one, or switch the value back to text." (zh: 「这个值需要一个表达式。请填写表达式,或把这个值切回文本。」). Every other refusal shows as before. ## Tests - New `studio-design/AutomationsPillar.problemsAgree-27799.test.tsx` (real `FlowPreview` + real `FlowInspector`, server double as above): - one judge: for the blank value the flow parse passes and the publish verdict's messages equal the cell's live spec refusal; - the cell and the Problems row both show the business sentence; the row's target names `node 'write'` and `config.fields.id`; the tab count is 1; - control: writing `record.id` clears the cell, and after the autosave Problems reads "No problems" again. - `FlowKeyValueField.valueEnvelope.test.tsx`: the blank-expression case now expects the business sentence (the malformed-template case still shows the spec's sentence). ## Acceptance notes - Seam, outside this card's surface: the lint finding for a flow node's config value carries no item path (`ExprIssue.path` is set for an edge `condition` and an object validation's `condition`/`when` only), so `_diagnostics` locates it by the `where` prose and the Problems row cannot select or reveal the node. Seam: `lint:ExprIssue.path (flow node value slots) → runtime:withPublishVerdict itemRelativeIssuePath → renderer:flow-problems serverTarget`. With a `flows[0].nodes[N].config.fields.KEY` path, the existing mapping places the row on the node with no ui change. Filed by the seat as #27828 (`domain:spec`, p3). - Studio's Problems list now also shows the draft's schema errors and other publish findings it never showed before (the metadata edit page already did). That is the intended reach, not a side effect. - Row 27's suggestion half (a "Triggering record" target, the object pre-filled, field pickers for keys) stays on #27521. ## Verification <details><summary>Repro on main (before)</summary> The measurement test, run on `86aaff32` before the change: cell alert = the spec's envelope sentence; 1 save, 0 refused; served verdict `valid: false` with the same message at the `where` locator; Problems showed the empty-state sentence, 0 rows. </details> <details><summary>Ablations (each through scripts/ablation-replace.mjs, restored to HEAD)</summary> - `diagnostics: flowPreviewDiagnostics` ablated → 2 of 3 new cases red (Problems stays empty). - the Problems-row business wording ablated → 1 red. - the value-cell business wording ablated → 2 red (the new case and the updated `valueEnvelope` case). </details> <details><summary>Local runs</summary> All at head `7c47a855`, under `scripts/pm/os-verify-lock.sh`: - `pnpm --filter @object-ui/app-shell run type-check` (both tsc programs) → exit 0. - `pnpm --dir ui exec vitest run` over `packages/app-shell/src/views/studio-design/` plus the 91 test files that name `FlowPreview`, `ProblemsPanel`, `flow-problems`, `FlowKeyValueField`, `flow-value-envelope`, `draftVerdict`, `FlowNodeConfigField` or `FlowNodeInspector` → Test Files 357 passed (357), Tests 3097 passed (3097). - `pnpm --dir ui exec vitest run packages/i18n/` → Test Files 90 passed (90), Tests 1380 passed, 13 skipped. - `pnpm check:new-line-citations` (ui) → 0 new citations, exit 0. - Not run locally, declared to CI: the whole Test UI shards and the rest of the lint farm. </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `7c47a855` derived 34 families; each run and recorded as `command :: exit code` before any pipe; all 34 exit 0, including `node scripts/check-sdui-manifest.mjs` and `pnpm --dir ui exec vitest run scripts/` (169 passed, 2 skipped files; run alone under the lock). `--ran` reconciliation: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CaycqDGtn6KeXgc8hp)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 165bc50 commit ddab8b9

12 files changed

Lines changed: 333 additions & 6 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/i18n': patch
4+
---
5+
6+
fix(studio): the flow Problems list says what the publish will refuse, and a blank expression reads "This value needs an expression"
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
On the Studio's Automations page, a flow's Problems list judged only the canvas structure, so an Update Records value written as an expression and left blank showed a refusal under the value while Problems said "No problems — this flow is structurally valid", and Publish then refused the flow.
12+
13+
The Problems list now reads the verdict the pending draft's read already carries, the one the step inspectors read and the one Publish enforces, so a value the publish refuses is listed, located by the server's own words naming the step and the field. The value cell and the Problems row say the same thing, in business words: "This value needs an expression. Write one, or switch the value back to text." Any other refusal is shown as before.

‎ui/packages/app-shell/src/views/metadata-admin/draftVerdict.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,22 @@ export function withServerOnlyErrors<T extends { path: string; message: string }
7878
return [...clientErrors, ...extra];
7979
}
8080

81+
/**
82+
* objectstack#27799 — a served verdict as the flat, severity-tagged list a
83+
* canvas preview's Problems list reads (`MetadataPreviewProps.diagnostics`):
84+
* the errors, then the warnings, each at the path the server wrote. A page
85+
* that keeps the verdict and draws a flow hands it this, so the Problems list
86+
* says what the publish will refuse, as the inspectors already do.
87+
*/
88+
export function verdictPreviewEntries(
89+
diagnostics: ServerDiagnostics | undefined,
90+
): Array<{ path?: string; message: string; severity: 'error' | 'warning' }> {
91+
return [
92+
...(diagnostics?.errors ?? []).map((e) => ({ path: e.path || undefined, message: e.message, severity: 'error' as const })),
93+
...(diagnostics?.warnings ?? []).map((w) => ({ path: w.path || undefined, message: w.message, severity: 'warning' as const })),
94+
];
95+
}
96+
8197
/**
8298
* objectstack#26858 — the served verdict on the body on screen, for a field
8399
* that edits one path of it. The page holding the draft read provides it

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import type { ScopeGroup } from './useFlowScope.js';
4949
import { FlowExprIssue } from './FlowExprIssue.js';
5050
import {
5151
ASSIGNMENT_ARRAY_FORM_PRESCRIPTION,
52+
isBlankExpressionRefusal,
5253
isEditableValueEnvelope,
5354
valueEnvelopeRefusal,
5455
writeValueEnvelope,
@@ -227,6 +228,12 @@ export interface FlowKeyValueFieldProps {
227228
toggleLabel: string;
228229
/** Placeholder of an expression row's source input. */
229230
expressionPlaceholder: string;
231+
/**
232+
* What the cell says where the spec refuses an expression left blank
233+
* (`isBlankExpressionRefusal`), in place of the envelope's wire shape.
234+
* The verdict stays the spec's; only its words change.
235+
*/
236+
blankExpressionMessage?: string;
230237
};
231238
/** The spec requires this map (objectui#10948): the label carries {@link RequiredMarker}. */
232239
required?: boolean;
@@ -371,6 +378,10 @@ export function FlowKeyValueField({
371378
const hole = valueMode === 'hole' && !expression;
372379
const pickerGroups = expression || hole ? (scopeGroups ?? []) : [];
373380
const refusal = envelopeSlot && writesMap ? valueRefusal(rowValue(row)) : null;
381+
const blankText = valueEnvelope?.blankExpressionMessage;
382+
const shownRefusal = refusal
383+
? [...new Set(refusal.map((m) => (blankText && isBlankExpressionRefusal(m) ? blankText : m)))]
384+
: null;
374385
return (
375386
<div key={row.id} className="space-y-1">
376387
{/*
@@ -436,9 +447,9 @@ export function FlowKeyValueField({
436447
</Button>
437448
)}
438449
</div>
439-
{refusal ? (
450+
{shownRefusal ? (
440451
<div className="space-y-0.5" role="alert">
441-
{refusal.map((message, i) => (
452+
{shownRefusal.map((message, i) => (
442453
<p key={i} className="text-[11px] leading-snug text-destructive">
443454
{message}
444455
</p>

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/FlowKeyValueField.valueEnvelope.test.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from '@objectstack/spec/automation';
1717
import { FlowNodeConfigField } from './FlowNodeConfigField';
1818
import type { FlowConfigField } from './flow-node-config';
19+
import { t } from '../i18n';
1920

2021
afterEach(cleanup);
2122

@@ -119,7 +120,11 @@ describe('assignment value cell — text or CEL expression (objectui#7588)', ()
119120
expect(screen.queryByRole('alert')).toBeNull();
120121
fireEvent.click(screen.getByRole('button', { name: TOGGLE }));
121122
expect(onCommit).toHaveBeenLastCalledWith({ empty: { dialect: 'cel', source: '' } });
122-
expect(screen.getByRole('alert').textContent).toContain(ASSIGNMENT_VALUE_ENVELOPE_REFUSAL);
123+
// objectstack#27799 — the spec's refusal of a blank expression, said in
124+
// business words rather than the envelope's wire shape.
125+
const alert = screen.getByRole('alert').textContent ?? '';
126+
expect(alert).toBe(t('engine.inspector.flowNode.kv.expressionNeeded'));
127+
expect(alert).not.toContain(ASSIGNMENT_VALUE_ENVELOPE_REFUSAL);
123128
});
124129

125130
it('names the legacy array form with the spec prescription, and an expression writes the map', () => {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale,
621621
? {
622622
toggleLabel: t('engine.inspector.flowNode.kv.asExpression', locale),
623623
expressionPlaceholder: t('engine.inspector.flowNode.kv.expressionPlaceholder', locale),
624+
blankExpressionMessage: t('engine.inspector.flowNode.kv.expressionNeeded', locale),
624625
}
625626
: undefined,
626627
// objectui#12101 — a map whose every value is a flow text slot (http

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/flow-value-envelope.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,23 @@ export function valueEnvelopeRefusal(value: unknown): string[] | null {
118118
const result = AssignmentValueSchema.safeParse(value);
119119
return result.success ? null : result.error.issues.map((issue) => issue.message);
120120
}
121+
122+
/**
123+
* The spec's refusal of an expression left blank, read once from the spec's
124+
* own judge rather than restated: the messages {@link valueEnvelopeRefusal}
125+
* gives an envelope in the slot's dialect whose `source` is empty.
126+
*/
127+
const BLANK_EXPRESSION_REFUSALS: ReadonlySet<string> = new Set(
128+
valueEnvelopeRefusal({ dialect: VALUE_DIALECT, source: '' }) ?? [],
129+
);
130+
131+
/**
132+
* Whether `message` is the spec's refusal of a blank expression in a value
133+
* slot, so an editor can say it in business words. The node's value cell and
134+
* the flow's Problems list both ask this of the one verdict they show (the
135+
* cell's live spec parse, the publish verdict the draft read serves), so the
136+
* two say the same thing.
137+
*/
138+
export function isBlankExpressionRefusal(message: string): boolean {
139+
return BLANK_EXPRESSION_REFUSALS.has(message);
140+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function targetLabel(
6262
title: `${p.target.source} → ${p.target.target}`,
6363
};
6464
}
65-
return { text: 'flow' };
65+
return p.location ? { text: p.location, title: p.location } : { text: 'flow' };
6666
}
6767

6868
export function ProblemsPanel({ problems, selectedKey, onSelectProblem, locale, runStatus, nodes = [] }: ProblemsPanelProps) {

‎ui/packages/app-shell/src/views/metadata-admin/previews/flow-problems.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { describeExprSite, flowNodeIds, nodeIdPositions, type ExprSite } from '.
3232
import { flowNodeDisplayName } from './flow-canvas-parts.js';
3333
import { t, tFormat } from '../i18n.js';
3434
import { uniqueId } from '../inspectors/unique-id.js';
35+
import { isBlankExpressionRefusal } from '../inspectors/flow-value-envelope.js';
3536

3637
/** What a problem points at on the canvas — drives badge placement + reveal. */
3738
export type FlowProblemTarget =
@@ -55,6 +56,14 @@ export interface FlowProblem {
5556
* e.g. every hop of a cycle. The badge + click-reveal still use `target`.
5657
*/
5758
highlight?: { nodeIds: string[]; edges: Array<{ source: string; target: string }> };
59+
/**
60+
* Where the server located a problem it could not place on an element: its
61+
* own locator, shown as the row's target in place of the bare flow. A
62+
* publish finding on a node's config value is located in words (`flow 'x' ·
63+
* node 'y' (update_record) … at config.fields.id`), and that is the only
64+
* place the row names the node and the field.
65+
*/
66+
location?: string;
5867
}
5968

6069
/** A server diagnostic entry (subset of the layered record's `_diagnostics`). */
@@ -269,12 +278,17 @@ export function buildFlowProblems({ nodes, edges, serverDiagnostics, variables,
269278
(serverDiagnostics ?? []).forEach((diag, i) => {
270279
const level: DiagnosticLevel = diag.severity === 'warning' ? 'warning' : 'error';
271280
const target = serverTarget(diag.path, nodes, edges);
281+
const location = target.kind === 'flow' ? pathSegments(diag.path).join('.') : '';
272282
problems.push({
273283
id: `server:${i}:${targetKey(target)}`,
274284
level,
275-
message: diag.message,
285+
// The words the node's value cell uses for the same refusal.
286+
message: isBlankExpressionRefusal(diag.message)
287+
? t('engine.inspector.flowNode.kv.expressionNeeded', locale)
288+
: diag.message,
276289
target,
277290
source: 'server',
291+
...(location ? { location } : {}),
278292
});
279293
});
280294

0 commit comments

Comments
 (0)