From 46f4e0e90689ecba879e922d0a18e7e34a764452 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 01:12:42 +0000 Subject: [PATCH] fix(app-shell): make the flow end node's Outcome a spec-derived select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `end.config.outcome` was declared as a free-text box whose placeholder printed `success · failure`. `EndConfigSchema.outcome` is a closed enum of `completed | refused` defaulting to `completed`, and `FlowNodeSchema` discriminates an `end` node's config through it, so both printed words are refused at parse. On a key with no dropdown that placeholder was the only vocabulary the form offered, so the author's most likely action produced a flow that fails to load. The control is now a `select` over exactly the spec's enum, declaring `defaultValue: 'completed'` so an unset key states what the runtime applies to it, with the invented placeholder deleted. Options and default are derived from the installed spec and reconciled against `EndConfigSchema` through zod's public `toJSONSchema`, so the declaration cannot rot at the next bump. The field's help names the cross-field rule `refused` carries (it requires a `message`), which this form has no typed control for and which stays authorable in Advanced. The two exact-count declaration pins move by one each, computed from the value standing on this branch point rather than from a number copied out of a card. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .changeset/9278-end-outcome-select.md | 50 +++++++ .../src/views/metadata-admin/i18n.ts | 6 +- ...FlowNodeInspector.declaredDefault.test.tsx | 124 +++++++++++++++++- .../inspectors/flow-node-config.ts | 24 +++- 4 files changed, 196 insertions(+), 8 deletions(-) create mode 100644 .changeset/9278-end-outcome-select.md diff --git a/.changeset/9278-end-outcome-select.md b/.changeset/9278-end-outcome-select.md new file mode 100644 index 0000000000..80242cfbd7 --- /dev/null +++ b/.changeset/9278-end-outcome-select.md @@ -0,0 +1,50 @@ +--- +'@object-ui/app-shell': patch +--- + +The flow `end` node's Outcome control becomes a select over the two outcomes the spec +accepts, and stops advertising two it refuses (objectui#9278). + +`packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts` declared the +`end` group's `outcome` key as a free-text box with `placeholder: 'success · failure'`. +`EndConfigSchema.outcome` is a closed enum of `completed | refused` defaulting to +`completed`, and `FlowNodeSchema` discriminates an `end` node's config through it — so +both printed words are refused at parse, not ignored at run time. Measured against the +installed `@objectstack/spec` (17.4.0), with the accepted row in the same output so the +refusals are a reading rather than a dead probe: + +``` +FlowNodeSchema config.outcome = "success" => REJECTED: Invalid option: expected one of "completed"|"refused" +FlowNodeSchema config.outcome = "failure" => REJECTED: Invalid option: expected one of "completed"|"refused" +FlowNodeSchema config.outcome = "completed" => ACCEPTED +EndConfigSchema.safeParse({}) => {"outcome":"completed"} +``` + +The placeholder was not a neutral hint. On a key with no dropdown it was the only +vocabulary the form offered, so the author's most likely action was to type one of the two +words printed in the box — and the flow then failed to load. This is Commandment #0 one +level down: the **values** are part of the contract too. + +The control is now a `select` whose options are exactly the spec's enum, declaring +`defaultValue: 'completed'` so an unset key states on the trigger what the runtime applies +to it, and the invented placeholder is gone. Both are derived from the installed spec, as +`FlowConfigField.defaultValue`'s doc comment requires of a declaration outside the +escalation ledger, and both are reconciled against `EndConfigSchema` in +`FlowNodeInspector.declaredDefault.test.tsx` — through zod's public `toJSONSchema` rather +than a respelled literal, so the claim cannot quietly rot at the next spec bump. The zh-CN +overlay gains the two option labels and the help line. + +`refused` carries a cross-field rule the same parse publishes and this form has no typed +control for: it requires a `message` saying why, as a `{token}` template, and `completed` +refuses one. That is named in the field's help rather than left for the author to discover +by a failed load; the key itself stays authorable through the Advanced block, which is +reachable even when a node carries no extra keys. Filed separately rather than fixed here. + +`patch`, not `minor`, and not breaking — measured on three axes. The published type +surface is unchanged: `flow-node-config` is not exported from `packages/app-shell`'s entry, +so the emitted `dist/index.d.ts` is untouched. No authored document changes meaning: no +metadata key is added or removed, and a stored value outside the new options still renders, +flagged deprecated, by the branch `FlowNodeConfigField` already had. The one capability +removed is typing an arbitrary string into this field — and every string that removes was +already refused by the loader, so nothing that worked stops working. The same reasoning +scored objectui#6830's select half on this file a `patch`. diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index d699dcfc55..a8b8b205dd 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -4103,7 +4103,11 @@ const FLOW_FIELD_ZH: Record> = { criteria: { label: '进入条件(旧)', help: '旧字段 —— 建议使用“进入条件”(condition)。' }, }, end: { - outcome: { label: '结果' }, + outcome: { + label: '结果', + help: '运行在此处如何结束。“已完成”是普通终态,也是省略该键时的取值。“已拒绝”把拒绝记为一等结果 —— 它是一次成功的评估,只是结论为否 —— 并要求给出拒绝理由 message({token} 模板),在“高级”中填写。', + opts: { completed: '已完成', refused: '已拒绝' }, + }, outputVariable: { label: '输出变量' }, }, decision: { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx index ef99494e06..eb965a671e 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx @@ -63,6 +63,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, cleanup } from '@testing-library/react'; +import { z } from 'zod'; // Mutable so a case can publish a server `configSchema` for one node type and // exercise the ONLINE field derivation, which is the other writer of @@ -85,7 +86,7 @@ import type { MetadataSelection } from '../preview-registry'; // instead of against a literal this file would then own a second copy of. // ⛔ The subpath is load bearing: `ApprovalEscalationSchema` is NOT on the // package root, where it reads `undefined` and any `.parse` on it throws. -import { ApprovalEscalationSchema } from '@objectstack/spec/automation'; +import { ApprovalEscalationSchema, EndConfigSchema, FlowNodeSchema } from '@objectstack/spec/automation'; /* ── The `meta/*` double (objectui#7307) ─────────────────────────────── * `FlowNodeInspector` renders `FlowReferenceField` for every reference-kind key @@ -588,9 +589,15 @@ describe('non-regression — a change that deletes the control must not pass thi describe('the declaration surface this card names', () => { /** - * The ten declaring fields, as `.`. Triage named this - * list the acceptance surface, so it is pinned: a PR that retires the - * property, or that adds an eleventh declaration, moves this line. + * The eleven declaring fields, as `.`. Triage named + * this list the acceptance surface, so it is pinned: a PR that retires the + * property, or that adds a twelfth declaration, moves this line. + * + * ⚠️ The number is NOT a constant to copy. objectui#9278 and objectui#9277 + * both move this line from the same base, so each computed it as the value + * standing here when it landed PLUS its own additions, and said in its PR + * body where it read the base from. Do the same rather than trusting either + * card's arithmetic — both were done assuming the other does not exist. * * Swept over the picker's node types plus the four that carry config but are * not offered in the picker (ADR-0031 import/export-only, and the legacy @@ -599,7 +606,7 @@ describe('the declaration surface this card names', () => { */ const OFF_PICKER_TYPES = ['boundary_event', 'parallel_gateway', 'join_gateway', 'legacy_action', 'notify']; - it('exactly ten fields declare a defaultValue, and these are they', () => { + it('exactly eleven fields declare a defaultValue, and these are they', () => { const swept = [...FLOW_NODE_TYPE_OPTIONS, ...OFF_PICKER_TYPES]; expect( FLOW_NODE_TYPE_OPTIONS.every((t) => swept.includes(t)), @@ -620,6 +627,7 @@ describe('the declaration surface this card names', () => { 'approval.maxRevisions', 'approval.onEmptyApprovers', 'boundary_event.boundaryConfig.eventType', + 'end.outcome', 'http_request.method', 'screen.mode', 'wait.waitEventConfig.eventType', @@ -660,12 +668,13 @@ describe('the declaration surface this card names', () => { expect( cases.map((c) => c.id).sort(), - 'the select-kind half of the declaration surface — seven of the ten', + 'the select-kind half of the declaration surface — eight of the eleven', ).toEqual([ 'approval.behavior', 'approval.escalation.action', 'approval.onEmptyApprovers', 'boundary_event.boundaryConfig.eventType', + 'end.outcome', 'http_request.method', 'screen.mode', 'wait.waitEventConfig.eventType', @@ -685,3 +694,106 @@ describe('the declaration surface this card names', () => { } }); }); + +/* ── objectui#9278: the `end` node's Outcome vocabulary ─────────────────────── + * `end.config.outcome` was a free-text box whose placeholder printed + * `success · failure`. `FlowNodeSchema` discriminates an `end` node's config + * through `EndConfigSchema`, whose `outcome` is a CLOSED enum of + * `completed | refused` — so BOTH printed words are refused at the door. On a + * key with no dropdown that placeholder was the only vocabulary the form + * offered, so the author's most likely action was to type one of the two words + * in the box, and the flow then failed to load. Commandment #0 one level down: + * the VALUES are part of the contract too. + * + * Every expectation below is DERIVED from the installed spec, through zod's + * public `toJSONSchema` rather than any wrapper internals — the `defaultValue` + * doc comment requires a declaration outside the escalation ledger to be + * derived from the spec rather than from taste, and a row that respelled the + * two words here would agree with itself while the form drifted. + * + * The parse rows are what make that derivation a READING rather than a dead + * probe, and they carry both signs in one output: every derived option is + * accepted at the door, and the two words the deleted placeholder printed are + * refused there. An accept-only loop would pass just as well against a schema + * that accepts everything. + * ─────────────────────────────────────────────────────────────────────────── */ +describe('the end node offers the outcomes the spec accepts (objectui#9278)', () => { + /** `{ enum, default }` for `EndConfigSchema.outcome`, read off the installed spec. */ + const outcomeSchema = ( + z.toJSONSchema(EndConfigSchema) as { + properties?: Record; + } + ).properties?.outcome; + const specOutcomes = (outcomeSchema?.enum ?? []) as string[]; + const specDefault = outcomeSchema?.default as string | undefined; + + /** + * The sibling an option requires, so an option's own row measures the OPTION. + * `refused` carries a cross-field rule — it requires a `message` — and a row + * that sent the bare key would read that refusal as "the enum rejects + * `refused`" and delete a value the contract declares. + */ + const siblingFor = (outcome: string) => + outcome === 'refused' ? { message: 'Refused: {record.name} is a confirmed duplicate' } : {}; + + const outcomeField = () => fieldsForNodeType('end').find((f) => f.id === 'outcome'); + + it('the spec still publishes the closed enum and the default this field derives from', () => { + // THE VACUITY GUARD. Every row below iterates `specOutcomes`; a spec that + // stopped publishing the enum — or a `toJSONSchema` shape this reader stops + // understanding — would make each of them pass over an EMPTY list, which is + // exactly the shape a derived expectation fails silently in. + expect(specOutcomes.length, 'EndConfigSchema.outcome publishes a closed enum').toBeGreaterThan(1); + expect(specOutcomes, 'and the default it applies to an omitted key is one of them').toContain(specDefault); + }); + + it('and FlowNodeSchema judges an end node through it — both signs, one reading', () => { + const verdict = (outcome: string) => + FlowNodeSchema.safeParse({ + id: 'e', + type: 'end', + label: 'E', + config: { outcome, ...siblingFor(outcome) }, + }).success; + + for (const outcome of specOutcomes) { + expect(verdict(outcome), `${outcome}: a derived option is accepted at the door`).toBe(true); + } + // The negative half, in the same reading — the two words the deleted + // placeholder printed, which is the whole defect this card is about. + expect(verdict('success'), '`success` — the old placeholder’s first word — is refused').toBe(false); + expect(verdict('failure'), '`failure` — its second — is refused').toBe(false); + }); + + it('the Outcome control is a select over exactly those outcomes, stating the spec default', () => { + const field = outcomeField(); + expect(field, 'the end node still has an Outcome field').toBeDefined(); + expect(field!.kind, 'an enum key is not authored as a free-text box').toBe('select'); + expect( + field!.options?.map((o) => o.value), + 'the offered vocabulary IS the spec enum, in the spec’s own order', + ).toEqual([...specOutcomes]); + expect(field!.defaultValue, 'and the form states the default the spec applies').toBe(specDefault); + // The invented vocabulary is gone rather than merely outvoted. A select + // needs no placeholder — the declared default draws in that slot — so any + // surviving string here would be a second, unchecked vocabulary. + expect(field!.placeholder, 'no invented placeholder survives on this field').toBeUndefined(); + }); + + it('and it RENDERS as a combobox on an end node, stating that default on the trigger', () => { + // The non-regression half this file keeps beside every table claim: the + // three rows above are all satisfied by an inspector that renders nothing. + renderInspector(draftWith('end', { config: {} })); + expect( + screen.queryByRole('combobox', { name: 'Outcome' }), + 'the Outcome control is rendered at all', + ).not.toBeNull(); + const declared = outcomeField()!.options?.find((o) => o.value === specDefault); + expect(declared, 'the declared default must be one of the offered options').toBeDefined(); + expect(triggerText('Outcome'), 'an unset key states the declared default').toBe(declared!.label); + expect( + triggerIsPlaceholder('Outcome'), + 'and states it as a placeholder, never as a selection the author made', + ).toBe(true); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts index 9aec028b1a..a49413ec42 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts @@ -470,7 +470,29 @@ const FLOW_NODE_CONFIG: Record = { }), ], end: [ - cfg('outcome', 'Outcome', 'text', { placeholder: 'success · failure' }), + // objectui#9278 — `outcome` is a CLOSED enum, and `FlowNodeSchema` + // discriminates an `end` node's config through `EndConfigSchema`, so a + // value outside it is refused at the door rather than ignored at run time. + // It was a free-text box whose placeholder printed `success · failure`: + // two words the parse contract refuses, and on a key with no dropdown that + // placeholder was the only vocabulary the form offered — so the author's + // most likely action was to type one of them, and the flow then failed to + // load. The options and the declared default are derived from the installed + // spec, as the `defaultValue` doc comment above requires of a declaration + // outside the escalation ledger, and reconciled against `EndConfigSchema` + // in `FlowNodeInspector.declaredDefault.test.tsx` so the claim cannot rot. + cfg('outcome', 'Outcome', 'select', { + options: [ + { value: 'completed', label: 'Completed' }, + { value: 'refused', label: 'Refused' }, + ], + defaultValue: 'completed', + // `refused` carries a cross-field rule the spec states and this form has + // no typed control for yet: it REQUIRES a `message` saying why, as a + // {token} template. Named here so picking it is not a one-click route to + // a flow that will not load; the key itself stays authorable in Advanced. + help: 'How the run ends here. "Completed" is the ordinary terminal and is what an omitted key applies. "Refused" records a first-class refusal — a successful evaluation that says no — and requires a message saying why (a {token} template), which is set in Advanced.', + }), cfg('outputVariable', 'Output variable', 'text', { placeholder: 'result' }), ], decision: [