diff --git a/.changeset/17306-screen-field-bound-help-lookup.md b/.changeset/17306-screen-field-bound-help-lookup.md new file mode 100644 index 0000000000..cc9b106459 --- /dev/null +++ b/.changeset/17306-screen-field-bound-help-lookup.md @@ -0,0 +1,94 @@ +--- +'@objectstack/spec': minor +'@objectstack/service-automation': minor +--- + +A flow screen field can now express a numeric bound, help text and a lookup target — spelled with the object field's own key names + + + +`ScreenFieldConfigSchema` was `.strict` over exactly +`name`/`label`/`type`/`required`/`options`/`defaultValue`/`placeholder`/`visibleWhen`, +so three ordinary authoring intents had **no expression at all**. They did not +degrade quietly — `max`, `helpText` and every lookup-target spelling were +refused BY NAME — but a loud refusal with no landing key is still a dead end, +and the reference app worked around all three in prose: a discount ceiling +interpolated into the `label` and the `placeholder` (with a comment explaining +why there was no `max`), and a `type: 'lookup'` field whose `placeholder` asked +a human to type a record id because the picker could not be pointed anywhere. + +Four keys land, and **their names are derived from `FieldSchema`, not invented** +— one platform, one field vocabulary, so a name learned on an object field means +the same thing on a screen field: + +| Key | Derived from | | +|:---|:---|:---| +| `min` / `max` | `FieldSchema.min` / `.max` | the bound pair | +| `inlineHelpText` | `FieldSchema.inlineHelpText` | help under the input — `FieldSchema` renames `help`/`helpText`/`hint`/`tooltip` onto it, so a screen-local `helpText` would have been a second contract for one question | +| `reference` | `FieldSchema.reference` | the object a `type: 'lookup'` field picks records from | + +**The bound is enforced, not advisory.** It rides to the client on +`ScreenFieldSpec` so the user is stopped at the input, **and** +`validateScreenInputs` re-checks it when the run resumes (`min_value` / +`max_value`, both already in the ADR-0114 D2 field-error catalog — no new error +code). A screen field's declared contract is the only contract behind it, so a +bound the dialog alone applied would be bypassed by any caller posting to +`resume` directly — the gap #4477 closed for `required`. + +That sentence needs no "when the value is a number" qualifier, because the +value SHAPE is checked first: on a `type: 'number'` field a present value that +is not a finite JSON number is refused with `invalid_type` (also already in the +catalog — still no new code), ⛔ **not coerced**. Before this, a bound pass that +compares numbers was satisfied by anything that never reached it, so `"25"` +under a `max` of `20` was conformant. One member of the open `type` vocabulary +is read as a value domain; every other widget hint stays open, and a bound on a +non-numeric field still constrains nothing. + +**Delivered with its rendering, not ahead of it.** The executor forwards all +four onto the wire and the Studio designer form offers all four as repeater +columns; `builtin-node-form-zod-ledger.test.ts` reconciles the two key sets +against the Zod in both directions, so a key declared here and absent from the +form fails that test rather than shipping as a field nobody can author. + +**BREAKING** in the accept-set sense, in TWO places — landing as `minor` on +both packages because the launch-window guard (`check-changeset-no-major`) +keeps breaking changes off `major` outside pre-mode, not because the narrowing +is small. Both were ruled (maintainer ruling A′, decision batch #130 item 1, +2026-09-13); this release is **not** purely additive. + +1. `reference` is **required** when `type` is `lookup`, as it is on an object + field. A picker with no target object resolves nothing — ADR-0078's own + example of silently-inert metadata — and a degraded shape that ships today + is not a reason to bend the contract to it. A stored flow with a bare + `lookup` screen field parsed before and does not now. There is **no lossless + conversion**: nothing in the metadata says which object the author meant, so + this is an ADR-0087 **semantic** migration entry — a structured TODO + (`screen-field-lookup-reference-required`) that names the flow and the field + for a human to answer — and ⛔ never a D2 conversion that would have to + invent a target. +2. A non-number submitted for a `type: 'number'` screen field is refused on + resume (`invalid_type`) instead of passing silently. A resume bag that was + accepted before can be refused now; it was never doing what its author + declared. + +Everything else is additive: the bound itself fires only on a field that +declares one, which nothing did before this release. + +The neighbouring spellings are refused **with their landing key** rather than +with a bare key list: `help`/`helpText`/`hint`/`tooltip` name `inlineHelpText`, +and `object`/`referenceTo`/`targetObject`/`lookupObject`/`relatedTo`/`target` +name `reference`. ⚠️ `object` means different things one level apart — on the +screen **node** it renames to `objectName`, on a screen **field** it can only +mean the lookup target — so it earns its own row on both. + +**One stale claim corrected in passing, because this change falsified it.** The +flows translation surface documented `help`'s exclusion as *"`ScreenFieldConfig` +declares nothing help-shaped at all"*, in `translation.zod.ts`'s guidance string +(which enumerated the old key set verbatim), its doc block, and +`i18n-resolver.ts`'s `FLOW_SCREEN_FIELD_COPY_KEYS`. The screen field now +declares `inlineHelpText`, so the copy is real. The exclusion **stands** — the +flows bundle still carries `label` and `placeholder` only, and growing that face +is a ruled step against the #7646 enumeration, not a resolver-side accretion — +but its reason is now stated as a not-yet instead of telling an author the field +has no help copy when it has. ⛔ No translation key was added and no resolver +behaviour moved. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 2d73170000..c65998223c 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -396,6 +396,88 @@ Two things worth knowing: `config.fields[N].visibleWhen` — but a predicate that *parses* and simply names the wrong field still shows up as a field that never hides. +#### A field's bound, help text and lookup target + +A screen field spells these three with **the object field's own key names** — +one platform, one field vocabulary, so a name you learned declaring an object +field means the same thing here: + +| Key | Type | What it does | +|:---|:---|:---| +| `min` / `max` | `number` | Numeric bound. Applied by the client at the input **and enforced server-side when the run resumes**. | +| `inlineHelpText` | `string` | Help text under the input. Unlike `placeholder`, it stays readable after the user types. | +| `reference` | `string` | Object whose records a `type: 'lookup'` field picks from, so the field renders a record picker. **Required** on a `lookup` field. | + +```typescript +{ + id: 'quote_terms', + type: 'screen', + config: { + fields: [ + { name: 'discount', label: 'Discount %', type: 'number', + min: 0, max: 20, + inlineHelpText: 'Above 20% the quote is refused — this is a hard ceiling.' }, + { name: 'resolved_by_article', label: 'Resolved by Article', type: 'lookup', + reference: 'crm_knowledge_article' }, + ], + }, +} +``` + +**The bound is not advice.** `min` / `max` ride to the client so the user is +stopped at the input, and `validateScreenInputs` re-checks them on resume, so a +caller that skips the dialog and posts to `resume` directly is refused too +(`min_value` / `max_value`, inside the run's `INVALID_SCREEN_INPUT`). A screen +field's declared contract is the only contract behind it — there is no object +schema to catch a bad bag downstream. + +That sentence carries no "if the value is a number" qualifier, because the +shape is checked first: on a `type: 'number'` field a submitted value that is +not a JSON number is **refused** on resume with `invalid_type`. `"25"` — the +same quantity as a *string* — is **not coerced** into `25` and does not reach +the bound; it is rejected as the wrong shape. The bound then compares numbers +only, which is all a bound can do. + +**What these keys refuse:** + +- **A bound that is not a number.** `max: '20'` is refused at parse time + (`invalid_type`). The bound is a closed constraint even though the `type` + beside it is an open widget hint. +- **The neighbouring spellings, by name and with their target.** `helpText`, + `help`, `hint` and `tooltip` are refused and named onto `inlineHelpText`; + `object`, `referenceTo`, `targetObject`, `lookupObject`, `relatedTo` and + `target` are refused and named onto `reference`. ⚠️ `object` means different + things one level apart: on the screen **node** it renames to `objectName` + (render that object's whole form), on a screen **field** it can only mean the + lookup target. +- **A non-string `inlineHelpText` or `reference`.** +- **A `lookup` field with no `reference`.** A picker with no target object + resolves nothing, which is ADR-0078's own example of silently-inert + metadata, so `reference` is **required** when `type` is `lookup` — as it is + on an object field. The refusal names the key and shows the spelling. + ⚠️ This is a **narrowing**: a stored flow declaring a bare `lookup` screen + field parsed before this release and does not now. There is no lossless + conversion — nothing in the metadata says which object the author meant — so + the migration chain carries it as a structured TODO + (`screen-field-lookup-reference-required`) naming the flow and the field, + not an automatic rewrite. + +**What they deliberately do NOT refuse:** + +- **A bound on a non-numeric field.** A screen field's `type` has no closed + vocabulary, so this schema cannot judge which types a bound is meaningful on. + A bound on a `text` field constrains nothing and is not an error — the shape + check above reads `type: 'number'` alone, never the presence of a bound. +- **An absent value.** A bound constrains a value that is present — an optional + bounded field left empty is conformant. Presence is `required`'s question. +- **A bound on a field the user was never shown.** Like `required`, a bound on a + hidden `visibleWhen` field does not fire — the client is the authority on what + was on screen. + +Not translatable yet: the flows translation bundle carries `label` and +`placeholder` per field, so `inlineHelpText` renders in the authored language +until that face grows a key for it. + **Screen (object form):** Set `config.objectName` diff --git a/content/docs/references/automation/builtin-node-config.mdx b/content/docs/references/automation/builtin-node-config.mdx index 7b06b7fd45..cb9348b103 100644 --- a/content/docs/references/automation/builtin-node-config.mdx +++ b/content/docs/references/automation/builtin-node-config.mdx @@ -227,6 +227,10 @@ Value the variable takes: a string (`{token}` flow interpolation — a sole toke | **defaultValue** | `any` | optional | Prefilled value (interpolates `{token}` templates) | | **placeholder** | `string` | optional | Input placeholder text | | **visibleWhen** | `string` | optional | CEL predicate controlling visibility, evaluated client-side | +| **min** | `number` | optional | Minimum accepted value (numeric fields); enforced on resume | +| **max** | `number` | optional | Maximum accepted value (numeric fields); enforced on resume | +| **inlineHelpText** | `string` | optional | Help text displayed below the field | +| **reference** | `string` | optional | Target object name (snake_case) whose records a `type: 'lookup'` field picks from; REQUIRED when `type` is `lookup` | --- @@ -245,6 +249,10 @@ Value the variable takes: a string (`{token}` flow interpolation — a sole toke | **defaultValue** | `any` | optional | Prefilled value (interpolates `{token}` templates) | | **placeholder** | `string` | optional | Input placeholder text | | **visibleWhen** | `string` | optional | CEL predicate controlling visibility, evaluated client-side | +| **min** | `number` | optional | Minimum accepted value (numeric fields); enforced on resume | +| **max** | `number` | optional | Maximum accepted value (numeric fields); enforced on resume | +| **inlineHelpText** | `string` | optional | Help text displayed below the field | +| **reference** | `string` | optional | Target object name (snake_case) whose records a `type: 'lookup'` field picks from; REQUIRED when `type` is `lookup` | ### Nested Shape: `ScreenFieldConfig.options[number]` diff --git a/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts b/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts index 97f63f8ff3..5d42633825 100644 --- a/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts +++ b/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts @@ -84,8 +84,12 @@ describe('unknown node config keys are rejected (#4277)', () => { // `visibleWhen` is edit-distance 4 against `nearestName`'s threshold of 3, // so this exact typo gets no did-you-mean. Printing the declared set is // what makes the diagnostic actionable regardless. + // #17306 added `min`/`max`/`inlineHelpText`/`reference`; the enumeration is + // pinned in full rather than sampled, so a key that arrives or vanishes on + // this surface has to be acknowledged here. expect(msg).toContain( - 'Declared here: name, label, type, required, options, defaultValue, placeholder, visibleWhen.', + 'Declared here: name, label, type, required, options, defaultValue, placeholder, ' + + 'min, max, inlineHelpText, reference, visibleWhen.', ); // …and this particular key has a documented incident, so it also carries // its tombstone (the UNKNOWN_KEY_GUIDANCE pattern). diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts index db5be8148e..a7a35921ee 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.ts @@ -45,6 +45,33 @@ import { judgeHeadlessScreen } from '../screen-input-contract.js'; * that parse is what makes the retirement audible to stored metadata. */ +/** + * The `fields[].reference` repeater column of the `screen` designer form: the + * object PICKER (that is what `xRef` marks) whose collected value is the target + * object's NAME — the same string `FieldSchema.reference` carries. + * + * Named rather than written inline at its `reference:` key, and the naming is + * load-bearing. `check:reference-carrier-shape` classifies the object HOLDING a + * `reference` key; here that holder is a JSON-Schema `properties` map, keyed by + * property NAME — the same class the gate excludes by shape for a `fields` map + * (N1) and for a field-key class map (N3), but a shape its position rules have + * no case for. Written inline, the holder resolved under neither reading and + * the gate refused to guess (exit 3 — correctly). None of its three site + * remedies is spellable here: the holder is not under `fields:`, its own `type` + * key holds a sub-schema rather than a string literal, and all twelve of its + * keys are `data/Field` authorable keys, so no key can prove it is not a field + * definition. A value reached through a name is UNJUDGED by the gate's stated + * predicate, which judges LITERALS only. That narrows the gate nowhere else in + * the tree; the missing `properties`-map rule is reported to the maintainer + * rather than patched from inside this PR. + */ +const LOOKUP_TARGET_COLUMN = { + type: 'string', + title: 'Lookup object', + xRef: { kind: 'object' }, + description: "Object whose records a `lookup` field picks from. Required on a `lookup` field — a picker with no target object resolves nothing.", +}; + export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext): void { // screen — server-side pass-through (input vars already injected by engine). engine.registerNodeExecutor({ @@ -98,6 +125,17 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext }, defaultValue: { title: 'Default', description: 'Prefilled value. Interpolates {var} references.' }, placeholder: { type: 'string', title: 'Placeholder' }, + // Declared in #17306 — the bound pair, the help text and the + // lookup target, spelled as the object field spells them. + // Offered here for the same reason `options`/`defaultValue`/ + // `placeholder` were in #4045: the executor forwards them, so + // a form that omitted them would leave the keys authorable + // only by hand. `builtin-node-form-zod-ledger.test.ts` + // reconciles this column set against the Zod both ways. + min: { type: 'number', title: 'Min', description: 'Minimum accepted value (numeric fields). Enforced when the run resumes.' }, + max: { type: 'number', title: 'Max', description: 'Maximum accepted value (numeric fields). Enforced when the run resumes.' }, + inlineHelpText: { type: 'string', title: 'Help text', description: 'Help text shown under the input. Unlike the placeholder, it stays readable once the user types.' }, + reference: LOOKUP_TARGET_COLUMN, visibleWhen: { type: 'string', title: 'Visible when', xExpression: 'expression' }, }, }, @@ -208,6 +246,15 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext options: f.options as Array<{ value: unknown; label: string }> | undefined, defaultValue: f.defaultValue !== undefined ? interpolate(f.defaultValue, variables, context) : undefined, placeholder: f.placeholder, + // #17306. `min`/`max` go on the wire for the client to apply at the + // input, and are re-checked server-side on resume + // (`validateScreenInputs`) so the bound is not dialog-only. + // `inlineHelpText`/`reference` are presentation and picker-target + // hints the client alone acts on. + min: f.min, + max: f.max, + inlineHelpText: f.inlineHelpText, + reference: f.reference, // Forwarded RAW — deliberately not interpolated. `visibleWhen` is a // predicate the client re-evaluates on every keystroke against the // values collected SO FAR; the server has no view of those, so diff --git a/packages/services/service-automation/src/screen-input-contract.test.ts b/packages/services/service-automation/src/screen-input-contract.test.ts new file mode 100644 index 0000000000..8d5bb4be21 --- /dev/null +++ b/packages/services/service-automation/src/screen-input-contract.test.ts @@ -0,0 +1,127 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The bound pair on the screen-input VALUE contract (#17306). + * + * `min` / `max` are declared on `ScreenFieldConfigSchema` and forwarded onto + * the `ScreenFieldSpec` the client renders — but a screen field's declared + * contract is the ONLY contract behind it, so a bound the dialog alone applied + * would be bypassed by any caller that posts to `resume` directly. That is the + * gap #4477 closed for `required`, and these pin the same closure for the + * bound: the client stops the user at the input, the server stops everyone. + * + * ⚠️ Ruling A′ (2026-09-13) closed the second half of that sentence. The bound + * pass compares numbers, so a caller posting a non-number satisfied it by + * never reaching it — `"25"` under a `max` of 20 was conformant. A present + * value for a `type: 'number'` field is now refused unless it is a finite JSON + * number (`invalid_type`, ⛔ not coerced), and the pin that recorded the old + * silence is inverted in place rather than removed. + */ + +import { describe, expect, it } from 'vitest'; + +import type { ScreenFieldSpec } from '@objectstack/spec/contracts'; + +import { validateScreenInputs } from './screen-input-contract.js'; + +const ALWAYS_VISIBLE = () => true; + +const discount: ScreenFieldSpec = { name: 'discount', type: 'number', min: 0, max: 20 }; + +describe('validateScreenInputs — the declared bound pair (#17306)', () => { + it('accepts a value inside the bound, and both endpoints (inclusive)', () => { + for (const value of [0, 5, 20]) { + expect(validateScreenInputs([discount], { discount: value }, ALWAYS_VISIBLE), `value ${value}`) + .toEqual([]); + } + }); + + it('refuses a value above `max` with the catalog code that mirrors the key', () => { + const issues = validateScreenInputs([discount], { discount: 25 }, ALWAYS_VISIBLE); + expect(issues).toHaveLength(1); + expect(issues[0]!.code).toBe('max_value'); + expect(issues[0]!.field).toBe('discount'); + expect(issues[0]!.message).toContain('20'); + }); + + it('refuses a value below `min`', () => { + const issues = validateScreenInputs([discount], { discount: -1 }, ALWAYS_VISIBLE); + expect(issues).toHaveLength(1); + expect(issues[0]!.code).toBe('min_value'); + }); + + // ── The cost direction: what the bound must NOT start refusing ─────────── + it('leaves an unbounded field alone — the historical pass-through is intact', () => { + const free: ScreenFieldSpec = { name: 'note', type: 'number' }; + expect(validateScreenInputs([free], { note: 999999 }, ALWAYS_VISIBLE)).toEqual([]); + }); + + it('does not fire on an ABSENT value — that is the `required` question, not this one', () => { + // An optional bounded field left empty is conformant. Reading absence as + // an out-of-bound zero would make every optional bound secretly required. + expect(validateScreenInputs([discount], {}, ALWAYS_VISIBLE)).toEqual([]); + }); + + // ── REVERSED (ruling A′, 2026-09-13): this pin used to assert the SILENCE ── + // It read 'does not fire on a non-numeric value — `type` has no closed + // vocabulary here', and asserted `[]` for `{ discount: 'twenty' }`. That + // silence was the string gap: under a `max` of 20 the string `"25"` satisfied + // the bound pass (which compares numbers) and no other pass looked at it, so + // the guarantee "skipping the dialog is refused too" held only for callers + // that already sent a number. The maintainer closed it — refuse, ⛔ never + // coerce — so the same input now yields an issue, and the assertion is + // inverted rather than deleted: a later widening that re-admits the string + // has to come back through this case. + it('refuses a non-number for a `type: \'number\'` field — the string gap is closed', () => { + const issues = validateScreenInputs([discount], { discount: 'twenty' }, ALWAYS_VISIBLE); + expect(issues).toHaveLength(1); + expect(issues[0]!.code).toBe('invalid_type'); + expect(issues[0]!.field).toBe('discount'); + // ⛔ Not coerced: a numeric STRING is the shape being refused, so it must + // not be read as the number it spells and then bound-checked. + const numericString = validateScreenInputs([discount], { discount: '25' }, ALWAYS_VISIBLE); + expect(numericString.map((i) => i.code)).toEqual(['invalid_type']); + // …and the non-finite numbers, which are not JSON numbers either. + for (const value of [Number.NaN, Number.POSITIVE_INFINITY]) { + expect(validateScreenInputs([discount], { discount: value }, ALWAYS_VISIBLE).map((i) => i.code), + `value ${String(value)}`).toEqual(['invalid_type']); + } + }); + + it('reads only `type: \'number\'` as a value domain — every other widget hint stays open', () => { + // The closure is one member of an open vocabulary, not a new rule that + // every `type` constrains its value. A bound on a non-numeric field still + // constrains nothing, exactly as `FieldSchema.min`/`.max` do. + const texty: ScreenFieldSpec = { name: 'discount', type: 'text', min: 0, max: 20 }; + expect(validateScreenInputs([texty], { discount: 'twenty' }, ALWAYS_VISIBLE)).toEqual([]); + const untyped: ScreenFieldSpec = { name: 'discount', min: 0, max: 20 }; + expect(validateScreenInputs([untyped], { discount: 'twenty' }, ALWAYS_VISIBLE)).toEqual([]); + }); + + it('leaves the shape check to `required` when the value is absent, and off a hidden field', () => { + // Absence is `required`'s question — reading it as a bad shape would make + // every optional numeric field secretly required. + expect(validateScreenInputs([discount], {}, ALWAYS_VISIBLE)).toEqual([]); + expect(validateScreenInputs([discount], { discount: null }, ALWAYS_VISIBLE)).toEqual([]); + const conditional: ScreenFieldSpec = { ...discount, visibleWhen: 'wantsDiscount == true' }; + expect(validateScreenInputs([conditional], { discount: 'twenty' }, () => false)).toEqual([]); + // …and the control, or the reading above proves nothing. + expect(validateScreenInputs([conditional], { discount: 'twenty' }, () => true)).toHaveLength(1); + }); + + it('does not fire on a field the user was never shown', () => { + // Same reason `required` does not: the client is the authority on what was + // on screen, and enforcing over a hidden field dead-ends the run (#3528). + const conditional: ScreenFieldSpec = { ...discount, visibleWhen: 'wantsDiscount == true' }; + expect(validateScreenInputs([conditional], { discount: 999 }, () => false)).toEqual([]); + expect(validateScreenInputs([conditional], { discount: 999 }, () => undefined)).toEqual([]); + // …and the control: visible ⇒ it fires, or the three readings above prove nothing. + expect(validateScreenInputs([conditional], { discount: 999 }, () => true)).toHaveLength(1); + }); + + it('still reports `required` and `unknown_field` alongside a bound violation', () => { + const fields: ScreenFieldSpec[] = [discount, { name: 'reason', type: 'text', required: true }]; + const issues = validateScreenInputs(fields, { discount: 99, sparkles: true }, ALWAYS_VISIBLE); + expect(issues.map((i) => i.code).sort()).toEqual(['max_value', 'required', 'unknown_field']); + }); +}); diff --git a/packages/services/service-automation/src/screen-input-contract.ts b/packages/services/service-automation/src/screen-input-contract.ts index c2f87e1370..66691fddf7 100644 --- a/packages/services/service-automation/src/screen-input-contract.ts +++ b/packages/services/service-automation/src/screen-input-contract.ts @@ -32,10 +32,12 @@ export interface ScreenInputIssue { field: string; /** * Which constraint the bag violated, from the field-level catalog - * (ADR-0114 D2) — `required` and `unknown_field`. Typed as `FieldErrorCode` - * rather than a local literal union for the reason `ActionParamIssue` gives: - * a screen field, an action param and a record column must not drift into - * three vocabularies for the same two conditions. + * (ADR-0114 D2) — `required`, `invalid_type`, `min_value`, `max_value` and + * `unknown_field`. Typed as `FieldErrorCode` rather than a local literal + * union for the reason `ActionParamIssue` gives: a screen field, an action + * param and a record column must not drift into three vocabularies for the + * same conditions. ⛔ Every member here is one the catalog ALREADY declares — + * #17306 added three conditions to this surface and no code to that catalog. * * NOT an `error.code` (ADR-0112 D1). These are FIELD-ADDRESSED validator * codes that ride inside the refusal's message; the refusal's own machine @@ -83,6 +85,8 @@ export function screenDeclaresInputContract(screen: ScreenSpec | undefined): boo * * Enforced, and nothing beyond it: * - `required` presence for every field the caller was actually asked for; + * - the VALUE SHAPE of a `type: 'number'` field (#17306); + * - the declared numeric bound pair `min` / `max` (#17306); * - undeclared keys. * * `visibleWhen` is resolved FIRST, by the caller-supplied {@link visibility} @@ -95,8 +99,30 @@ export function screenDeclaresInputContract(screen: ScreenSpec | undefined): boo * degradation is loud rather than silent. Its KEY stays accepted either way — * the author declared it, so it is never "undeclared". * - * Value SHAPE (`type`) is out of scope here: a screen field's `type` is a - * widget hint with no closed vocabulary, unlike an action param's field type. + * Value SHAPE (`type`) is out of scope here with ONE closed exception, and the + * exception is the maintainer's (ruling A′, 2026-09-13): a screen field's + * `type` is an open widget hint, so no member of it constrains a value — + * EXCEPT `'number'`, whose meaning is not a rendering preference but a value + * domain the bound pair beside it already assumes. A present value for a + * `type: 'number'` field that is not a finite JSON number is refused with + * `invalid_type`, the catalog's existing member for a value of the wrong + * primitive type (ADR-0114 D2 — no new code, and deliberately not + * `invalid_number`, which names a PARSE failure rather than a type mismatch). + * It is refused, ⛔ never coerced: `"25"` is the author's client sending the + * wrong shape, and quietly reading it as `25` would make the bound's verdict + * depend on a coercion no part of this contract declares. + * + * That closes the gap the bound pass alone left open. The bound pass compares + * numbers, so before this every non-number slipped past BOTH — under a `max` + * of 20, `"25"` was conformant. The refusal now lands on the shape, and the + * bound pass is left comparing numbers only, which is all it can do. + * + * ⚠️ Narrow on purpose in two directions. It keys off `type: 'number'`, not + * off the presence of a bound: a bound declared on a non-numeric field still + * constrains nothing (`FieldSchema.min`/`.max` are unconditioned on `type` + * too), and no other widget hint is read as a value domain. And it is a + * PRESENCE-conditioned check, like the bound: an absent value is `required`'s + * question, and a field the caller was never shown is nobody's. */ export function validateScreenInputs( fields: readonly ScreenFieldSpec[], @@ -123,6 +149,56 @@ export function validateScreenInputs( }); } + // Value shape for a `type: 'number'` field (#17306, ruling A′). Its own pass, + // BEFORE the bound: a value that is not a number cannot be compared against + // one, so the shape is the first thing true or false about it. A caller that + // posts `"25"` under a `max` of 20 gets one issue naming the shape, not a + // silent pass and not a bound verdict computed on a string. + for (const field of declared.values()) { + if (field.type !== 'number') continue; + const value = bag[field.name]; + // Absent is `required`'s question (see the bound pass below for the same + // split), and a field the user was never shown is neither's. + if (!isPresent(value)) continue; + if (field.visibleWhen != null && String(field.visibleWhen).trim() !== '') { + if (visibility(field) !== true) continue; + } + if (typeof value === 'number' && Number.isFinite(value)) continue; + issues.push({ + field: field.name, + code: 'invalid_type', + message: `Screen field "${field.name}" must be a number`, + }); + } + + // Bound pair (#17306). Separate pass from `required` on purpose: a bound + // constrains a value that IS present, so an absent one is the `required` + // question and never this one — an optional bounded field left empty is + // conformant. A field the user was never shown is not bound-checked either, + // for the reason `required` is not: the client is the authority on what was + // on screen, and #3528's dead-end is the cost of getting that backwards. + for (const field of declared.values()) { + const value = bag[field.name]; + if (typeof value !== 'number' || !Number.isFinite(value)) continue; + if (field.visibleWhen != null && String(field.visibleWhen).trim() !== '') { + if (visibility(field) !== true) continue; + } + if (typeof field.min === 'number' && value < field.min) { + issues.push({ + field: field.name, + code: 'min_value', + message: `Screen field "${field.name}" must be at least ${field.min}`, + }); + } + if (typeof field.max === 'number' && value > field.max) { + issues.push({ + field: field.name, + code: 'max_value', + message: `Screen field "${field.name}" must be at most ${field.max}`, + }); + } + } + for (const key of Object.keys(bag)) { if (declared.has(key)) continue; issues.push({ diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index 2af06dd3bb..9dc1216ccf 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -201,6 +201,7 @@ "RetryPolicySchema (const)", "SCHEDULE_ORGANIZATION_KEY (const)", "SCHEMALESS_NODE_CONFIG_SCHEMAS (const)", + "SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED (const)", "STRUCTURAL_CONDITION_SHAPE_REFUSAL (const)", "ScheduleOrganization (type)", "ScheduleOrganizationSchema (const)", diff --git a/packages/spec/authorable-surface/automation.json b/packages/spec/authorable-surface/automation.json index 39631c0a11..3db3517c88 100644 --- a/packages/spec/authorable-surface/automation.json +++ b/packages/spec/authorable-surface/automation.json @@ -306,10 +306,14 @@ "automation/ScreenConfig:title", "automation/ScreenConfig:waitForInput", "automation/ScreenFieldConfig:defaultValue", + "automation/ScreenFieldConfig:inlineHelpText", "automation/ScreenFieldConfig:label", + "automation/ScreenFieldConfig:max", + "automation/ScreenFieldConfig:min", "automation/ScreenFieldConfig:name", "automation/ScreenFieldConfig:options", "automation/ScreenFieldConfig:placeholder", + "automation/ScreenFieldConfig:reference", "automation/ScreenFieldConfig:required", "automation/ScreenFieldConfig:type", "automation/ScreenFieldConfig:visibleWhen", diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index 7d6a2788b8..26082654c0 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -196,6 +196,7 @@ "RetryPolicySchema": "src/shared/retry-policy.zod.ts#RetryPolicySchema (const)", "SCHEDULE_ORGANIZATION_KEY": "src/automation/schedule-organization.zod.ts#SCHEDULE_ORGANIZATION_KEY (const)", "SCHEMALESS_NODE_CONFIG_SCHEMAS": "src/automation/schemaless-node-config.zod.ts#SCHEMALESS_NODE_CONFIG_SCHEMAS (const)", + "SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED": "src/automation/builtin-node-config.zod.ts#SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED (const)", "STRUCTURAL_CONDITION_SHAPE_REFUSAL": "src/automation/flow-node-expression-paths.ts#STRUCTURAL_CONDITION_SHAPE_REFUSAL (const)", "ScheduleOrganization": "src/automation/schedule-organization.zod.ts#ScheduleOrganization (type)", "ScheduleOrganizationSchema": "src/automation/schedule-organization.zod.ts#ScheduleOrganizationSchema (const)", diff --git a/packages/spec/liveness/translation.json b/packages/spec/liveness/translation.json index c2d6a1adde..85348ffad7 100644 --- a/packages/spec/liveness/translation.json +++ b/packages/spec/liveness/translation.json @@ -109,7 +109,7 @@ "note": "Per-screen heading + per-field copy, keyed by `FlowNode.id` / `ScreenFieldConfig.name` — the identifiers the client already holds as `ScreenSpec.nodeId` / `ScreenFieldSpec.name`. `planned` with its container: the screen-flow runner half is a downstream objectui card. Deeper conventions (`screens..title`, `screens..fields..{label,placeholder}`) are governed by that runner, not by ledger rows — the one-drill-level boundary this ledger's type note states." } }, - "note": "[#7646] Contract-first spec half of the screen-flow localization split, and `planned` is the honest status rather than `live` or `dead`: `dead` means declared with no consumer and no plan, while this group was ruled into the vocabulary by the maintainer specifically so the runner half could be built against it (the same ruling fixes the boundary — runner chrome, Cancel/Submit, stays in the console's own message catalog, NOT here). Addressing is measured against what the runner already holds: `flows..screens.` — the node id reaches the client verbatim as `ScreenSpec.nodeId` (packages/spec/src/contracts/automation-service.ts:138), which is also what correlates a resume back to its pause point — and `.fields.` (packages/spec/src/automation/builtin-node-config.zod.ts:382, forwarded as `ScreenFieldSpec.name`). Key face measured against `ScreenFieldConfigSchema`, not mirrored from the report: `label` + `placeholder` are declared, `help` is NOT — the screen field has no help-shaped key at all, so declaring it would parse clean and translate nothing, the ADR-0078 shape #6080 kept out of the page-component face; it rides `guidance` on the field surface instead, alongside `options`, which cannot be addressed by a value-keyed map because `ScreenFieldConfig.options[].value` is unconstrained. Flip to `live` with an objectui screen-flow-runner evidence pointer when the downstream consumer card lands; the resolver-side helper (a `FLOW_SCREEN_COPY_KEYS` sibling of `PAGE_COMPONENT_COPY_KEYS` in packages/spec/src/system/i18n-resolver.ts) is deliberately NOT in this change — #7634 was in flight on that file." + "note": "[#7646] Contract-first spec half of the screen-flow localization split, and `planned` is the honest status rather than `live` or `dead`: `dead` means declared with no consumer and no plan, while this group was ruled into the vocabulary by the maintainer specifically so the runner half could be built against it (the same ruling fixes the boundary — runner chrome, Cancel/Submit, stays in the console's own message catalog, NOT here). Addressing is measured against what the runner already holds: `flows..screens.` — the node id reaches the client verbatim as `ScreenSpec.nodeId` (packages/spec/src/contracts/automation-service.ts:138), which is also what correlates a resume back to its pause point — and `.fields.` (packages/spec/src/automation/builtin-node-config.zod.ts:382, forwarded as `ScreenFieldSpec.name`). Key face measured against `ScreenFieldConfigSchema`, not mirrored from the report: `label` + `placeholder` are declared, `help` is NOT — but ⚠️ no longer for its original reason: #17306 gave the screen field `ScreenFieldConfig.inlineHelpText`, so the help copy is REAL and what is missing is only THIS face's translation key for it. Growing that face is a ruled step against the #7646 enumeration, not a resolver-side accretion, so until it lands a `help` entry here would still parse clean and translate nothing — the ADR-0078 shape #6080 kept out of the page-component face, on a not-yet reason rather than an absent-key one; it rides `guidance` on the field surface instead, alongside `options`, which cannot be addressed by a value-keyed map because `ScreenFieldConfig.options[].value` is unconstrained. Flip to `live` with an objectui screen-flow-runner evidence pointer when the downstream consumer card lands; the resolver-side helper (a `FLOW_SCREEN_COPY_KEYS` sibling of `PAGE_COMPONENT_COPY_KEYS` in packages/spec/src/system/i18n-resolver.ts) is deliberately NOT in this change — #7634 was in flight on that file." }, "settings": { "status": "live", diff --git a/packages/spec/src/api/automation-api.zod.ts b/packages/spec/src/api/automation-api.zod.ts index 0d57bc4ca3..c2abbdd6e1 100644 --- a/packages/spec/src/api/automation-api.zod.ts +++ b/packages/spec/src/api/automation-api.zod.ts @@ -257,6 +257,14 @@ const screenFieldSpecShape = () => z.object({ })).optional().describe('Closed-enum options for select-style fields'), defaultValue: z.unknown().optional(), placeholder: z.string().optional(), + min: z.number().optional() + .describe('Minimum accepted value (numeric fields). Applied by the client at the input and enforced server-side on resume'), + max: z.number().optional() + .describe('Maximum accepted value (numeric fields). Applied by the client at the input and enforced server-side on resume'), + inlineHelpText: z.string().optional() + .describe('Help text rendered under the input. Unlike `placeholder`, it survives the user typing'), + reference: z.string().optional() + .describe("Object whose records a `type: 'lookup'` field picks from. Absent = no picker target to resolve"), visibleWhen: z.string().optional().describe( 'Conditional-visibility predicate, evaluated by the CLIENT against the ' + 'screen\'s live collected values - bare CEL over the screen\'s own field ' diff --git a/packages/spec/src/automation/builtin-node-config.test.ts b/packages/spec/src/automation/builtin-node-config.test.ts index 0bfab49209..5d690ac7ea 100644 --- a/packages/spec/src/automation/builtin-node-config.test.ts +++ b/packages/spec/src/automation/builtin-node-config.test.ts @@ -28,6 +28,7 @@ import { DeleteRecordConfigSchema, GetRecordConfigSchema, MapConfigSchema, + SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED, ScreenConfigSchema, ScreenFieldConfigSchema, UpdateRecordConfigSchema, @@ -245,6 +246,158 @@ describe('ScreenConfigSchema / ScreenFieldConfigSchema — strict as of #4001 }); }); +describe('ScreenFieldConfigSchema — the bound pair, help text and lookup target (#17306)', () => { + const BASE = { name: 'discount', type: 'number' }; + + // ── Direction 1: the three intents are now expressible ────────────────── + it('accepts the bound pair, the help text and the lookup target', () => { + expect(ScreenFieldConfigSchema.safeParse({ ...BASE, min: 0, max: 20 }).success).toBe(true); + expect(ScreenFieldConfigSchema.safeParse({ ...BASE, inlineHelpText: 'Ceiling is 20%.' }).success).toBe(true); + expect(ScreenFieldConfigSchema.safeParse({ + name: 'article', type: 'lookup', reference: 'crm_knowledge_article', + }).success).toBe(true); + }); + + it('reproduces the reference app\'s two prose workarounds as declared metadata', () => { + // hotcrm `quote_generation`: the discount ceiling lived in the label and + // the placeholder because no key carried it. + expect(ScreenFieldConfigSchema.safeParse({ + name: 'discount', type: 'number', label: 'Discount', min: 0, max: 20, + inlineHelpText: 'Above 20% the quote is refused — this is a hard ceiling.', + }).success).toBe(true); + // hotcrm `close_case`: the picker asked a human to type a record id. + expect(ScreenFieldConfigSchema.safeParse({ + name: 'resolved_by_article', type: 'lookup', label: 'Resolved by Article', + reference: 'crm_knowledge_article', + }).success).toBe(true); + }); + + // ── REVERSED (ruling A′, 2026-09-13): this pin used to assert the OPPOSITE ── + // It read `leaves a \`lookup\` field with no \`reference\` parsing, as it did + // before`, and its comment argued that shipped flows declare a bare `lookup` + // so refusing one would be a breaking change. The maintainer ruled the other + // way: ADR-0078's own example of silently-inert metadata IS a `lookup` with + // no `reference`, and a degraded shipped twin is not a reason to bend the + // contract. The assertion is inverted in place rather than deleted, so a + // later re-widening has to come back through this case. + it('refuses a `lookup` field with no `reference` — and names the key', () => { + const r = ScreenFieldConfigSchema.safeParse({ name: 'article', type: 'lookup' }); + expect(r.success).toBe(false); + const issue = r.error?.issues.find((i) => i.path[0] === 'reference'); + expect(issue, 'the refusal must be ADDRESSED to `reference`').toBeDefined(); + expect(issue!.message).toBe(SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED); + // The wording is the contract an author meets: it must name the key and + // show the spelling, and ⛔ must not carry a tracker id (check:doc-authoring). + expect(SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED).toContain('`reference`'); + expect(SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED).not.toMatch(/#\d{3,5}\b/); + // An empty or blank target is the same absence, not a declared one. + for (const blank of ['', ' ']) { + expect(ScreenFieldConfigSchema.safeParse({ name: 'article', type: 'lookup', reference: blank }).success, + `reference: ${JSON.stringify(blank)}`).toBe(false); + } + }); + + it('requires it on `lookup` ONLY — every other widget hint is untouched', () => { + // The requirement reads one member of an open `type` vocabulary. A field + // with no `type`, or any other one, still parses with no `reference`. + for (const type of [undefined, 'text', 'number', 'select']) { + expect(ScreenFieldConfigSchema.safeParse({ name: 'article', ...(type ? { type } : {}) }).success, + `type: ${String(type)}`).toBe(true); + } + }); + + it('keeps `.shape` enumerable — the refinement did not change the key set', () => { + // `superRefine` is a CHECK, not a wrapper: the ledger test and the pin + // below both read `.shape`, and a ZodEffects here would break them + // silently by making the key set unreadable rather than wrong. + expect(Object.keys((ScreenFieldConfigSchema as unknown as z.ZodObject).shape)) + .toContain('reference'); + }); + + // ── Direction 2: what must STILL be refused ───────────────────────────── + it('still refuses an undeclared key by name — the strict guard did not widen past these four', () => { + const message = unknownKeyMessage(ScreenFieldConfigSchema, { ...BASE, sparkles: true }); + expect(message).toContain('this screen field'); + expect(message).toContain('`sparkles`'); + }); + + it('pins the declared key set exactly — four added, nothing else', () => { + // A `.strict` object that quietly grew a fifth key is the regression this + // catches; the enumeration is the only thing that can see it. + const declared = Object.keys( + (ScreenFieldConfigSchema as unknown as z.ZodObject).shape, + ).sort(); + expect(declared).toEqual([ + 'defaultValue', 'inlineHelpText', 'label', 'max', 'min', 'name', + 'options', 'placeholder', 'reference', 'required', 'type', 'visibleWhen', + ]); + }); + + it('refuses a bound that is not a number', () => { + // The bound is a closed constraint even though the `type` beside it is an + // open widget hint — a string ceiling is the shape that would otherwise + // reach the client and compare as text. + const r = ScreenFieldConfigSchema.safeParse({ ...BASE, max: '20' }); + expect(r.success).toBe(false); + expect(r.error?.issues.some((i) => i.code === 'invalid_type')).toBe(true); + expect(ScreenFieldConfigSchema.safeParse({ ...BASE, min: 'zero' }).success).toBe(false); + }); + + // A lookup target is the target object's NAME, so every non-string SHAPE has to + // be refused, not just the array this once spelled inline. Tabled for two + // reasons. It widens the pin — `{ object: 'x' }` is the exact carrier shape + // #13053 was filed for, and it was untested here. And it is the only spelling + // available: `check:reference-carrier-shape` judges a `reference` whose value + // is a LITERAL, and it could place THIS holder under neither of its readings + // (`{ ...BASE, reference: … }` is a spread plus one key that `data/Field` + // does declare), so it refused rather than guess. Its three site remedies all + // make the refusal WORSE here: giving the holder a FieldType `type` or a + // `fields:` parent turns a rejection fixture into a reported finding, and + // `ScreenFieldConfig`'s twelve keys are every one of them `data/Field` keys, + // so none can prove the holder is not a field definition. Reaching the value + // through a name puts it in the population the gate documents as unjudged — + // it judges literals — while the assertion below gets STRICTER, not weaker. + const NON_STRING_LOOKUP_TARGETS: readonly unknown[] = [['a'], { object: 'crm_account' }, 42, true]; + + it('refuses help text and a lookup target that are not strings', () => { + expect(ScreenFieldConfigSchema.safeParse({ ...BASE, inlineHelpText: 42 }).success).toBe(false); + for (const target of NON_STRING_LOOKUP_TARGETS) { + const r = ScreenFieldConfigSchema.safeParse({ ...BASE, reference: target }); + expect(r.success, `reference: ${JSON.stringify(target)} must be refused`).toBe(false); + expect( + r.error?.issues.some((i) => i.path[0] === 'reference' && i.code === 'invalid_type'), + `reference: ${JSON.stringify(target)} must be refused ON \`reference\`, not incidentally`, + ).toBe(true); + } + }); + + // ── Direction 3: the neighbouring spellings are refused WITH their target ─ + it('sends the four help spellings to `inlineHelpText`, the object field\'s own key', () => { + for (const spelling of ['help', 'helpText', 'hint', 'tooltip']) { + const message = unknownKeyMessage(ScreenFieldConfigSchema, { ...BASE, [spelling]: 'x' })!; + expect(message, `${spelling} must name its landing key`).toContain('`inlineHelpText`'); + } + }); + + it('sends every lookup-target spelling to `reference`', () => { + for (const spelling of ['object', 'referenceTo', 'targetObject', 'lookupObject', 'relatedTo', 'target']) { + const message = unknownKeyMessage(ScreenFieldConfigSchema, { + name: 'article', type: 'lookup', [spelling]: 'crm_knowledge_article', + })!; + expect(message, `${spelling} must name its landing key`).toContain('`reference`'); + } + }); + + it('keeps the two levels apart: `object` means `objectName` on the node, `reference` on the field', () => { + // The same spelling, one level apart, with different answers — the trap the + // alias rows exist to disambiguate. + expect(unknownKeyMessage(ScreenConfigSchema, { object: 'showcase_task' })) + .toContain('`object` → `objectName`'); + expect(unknownKeyMessage(ScreenFieldConfigSchema, { name: 'a', object: 'showcase_task' })) + .toContain('`object` → `reference`'); + }); +}); + describe('MapConfigSchema — strict as of #4001 批 9', () => { it('accepts every declared key', () => { expect(MapConfigSchema.parse({ diff --git a/packages/spec/src/automation/builtin-node-config.zod.ts b/packages/spec/src/automation/builtin-node-config.zod.ts index 7d8fae16a0..e6428c9721 100644 --- a/packages/spec/src/automation/builtin-node-config.zod.ts +++ b/packages/spec/src/automation/builtin-node-config.zod.ts @@ -377,6 +377,23 @@ export type DeleteRecordConfigParsed = z.infer; * `ScreenSpec` the client renders). `visibleWhen` is forwarded RAW — the client * re-evaluates it against the values collected so far (#3528). */ +/** + * What a `type: 'lookup'` screen field with no target is told (maintainer + * ruling A′, 2026-09-13). Named rather than inlined for the reason every + * refusal string on this surface is: the wording is the contract an author + * meets, and a test pins it. + * + * It names the KEY, not just the condition: a picker with no target object is + * the exact shape ADR-0078 uses as its own example of silently-inert metadata + * — it parses, nothing marks it, and the field renders a picker that can + * resolve nothing. ⛔ No tracker id in this text: it is printed AT the author, + * who has no access to one (`check:doc-authoring`). + */ +export const SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED = + "A `type: 'lookup'` screen field must declare `reference` — the name of the object whose records the picker " + + "offers (e.g. `reference: 'crm_knowledge_article'`). Without it the field renders a picker with nothing to " + + 'resolve.'; + export const ScreenFieldConfigSchema = lazySchema(() => strictObject({ surface: 'this screen field', history: BUILTIN_NODE_CONFIG_HISTORY, @@ -400,6 +417,27 @@ export const ScreenFieldConfigSchema = lazySchema(() => strictObject({ */ visible: 'visibleWhen', showWhen: 'visibleWhen', + /** + * The three intents #17306 added land on `FieldSchema`'s OWN spellings — + * one platform, one field vocabulary, so an author who has declared an + * object field already knows these names. These renames are the same ones + * `FieldSchema` carries for the same targets (`field.zod.ts`), repeated + * here because an alias table is per-shape: a spelling this surface does + * not answer is refused with the bare key list, and the neighbouring- + * surface spellings are exactly what an author arrives holding. + */ + help: 'inlineHelpText', helpText: 'inlineHelpText', hint: 'inlineHelpText', tooltip: 'inlineHelpText', + relatedTo: 'reference', referenceTo: 'reference', target: 'reference', + targetObject: 'reference', lookupObject: 'reference', + /** + * `object` is NOT a `FieldSchema` alias — it earns a row here because this + * surface has two levels and they mean different things. On the screen NODE + * `object` renames to `objectName` (render this object's whole form); on a + * screen FIELD it can only mean the lookup target, which is `reference`. + * The report that filed #17306 reached for `object` at this level, so the + * spelling is measured, not hypothetical. + */ + object: 'reference', }, }, { /** Field name — an item with an empty name is dropped. */ @@ -424,6 +462,74 @@ export const ScreenFieldConfigSchema = lazySchema(() => strictObject({ placeholder: z.string().optional().describe('Input placeholder text'), /** Bare-CEL predicate the client re-evaluates as values change (#3528). */ visibleWhen: z.string().optional().describe('CEL predicate controlling visibility, evaluated client-side'), + /** + * Numeric bound pair (#17306), spelled as `FieldSchema` spells it. + * + * Forwarded into the `ScreenFieldSpec` the client renders AND enforced + * server-side on resume (`validateScreenInputs`, `min_value` / `max_value`): + * a screen field's declared contract is the ONLY contract behind it — there + * is no object schema to catch a bad bag downstream — so a bound that lived + * in the dialog alone would be bypassed by any caller that posts to `resume` + * directly, which is the gap #4477 closed for `required`. + * + * The resume check compares numbers, and a caller that posts a non-number no + * longer slips past it by never reaching it: on a `type: 'number'` field a + * present value that is not a finite JSON number is refused first + * (`invalid_type`), ⛔ not coerced (maintainer ruling A′, 2026-09-13). So + * "a caller that skips the dialog is refused too" holds with no qualifier. + * + * Unconditioned on `type` in the SHAPE, exactly as `FieldSchema.min` / + * `.max` are: a screen field's `type` is an open widget hint, so this schema + * cannot judge which types a bound is meaningful on. A bound declared on a + * non-numeric field still constrains nothing the client renders — the shape + * refusal above reads `'number'` alone, not the presence of a bound. + */ + min: z.number().optional().describe('Minimum accepted value (numeric fields); enforced on resume'), + max: z.number().optional().describe('Maximum accepted value (numeric fields); enforced on resume'), + /** + * Help text under the input (#17306) — `FieldSchema`'s spelling for the same + * intent, which is why it is not `helpText`: `FieldSchema` renames that (and + * `help` / `hint` / `tooltip`) onto `inlineHelpText`, so a screen field that + * declared `helpText` would make the platform answer one question with two + * names. Distinct from `placeholder`, which is the IN-INPUT hint and is gone + * the moment the user types — the carrier the reference app was forced to + * overload for a constraint that has to stay readable. + */ + inlineHelpText: z.string().optional().describe('Help text displayed below the field'), + /** + * Lookup target (#17306) — the object whose records a `type: 'lookup'` screen + * field picks from, spelled as `FieldSchema.reference` spells it. + * + * Optional in the SHAPE and REQUIRED on `type: 'lookup'` by the refinement + * below, which is how `FieldSchema` binds it too: the key is meaningless on + * a `text` field, so the requirement is conditional rather than a + * `z.string()` every screen field would have to carry. + * + * ⚠️ This REVERSES the optionality this card first shipped. The argument for + * optional was that shipped flows declare a bare `lookup` screen field (the + * reference app's own "Resolved by Article" is one) and refusing them breaks + * metadata that parses today. The maintainer ruled the other way (A′, + * 2026-09-13): ADR-0078's own example of silently-inert metadata IS a + * `lookup` with no `reference`, and a degraded shipped twin is not a reason + * to bend the contract to it. A stored bare lookup has NO lossless + * conversion — nothing in the metadata says which object the author meant — + * so this is an ADR-0087 SEMANTIC migration entry (a structured TODO naming + * the flow and the field), ⛔ never a D2 conversion that would have to invent + * a target. + */ + reference: z.string().optional().describe("Target object name (snake_case) whose records a `type: 'lookup'` field picks from; REQUIRED when `type` is `lookup`"), +}).superRefine((field, ctx) => { + // Conditional requirement, not a shape change: the key set is unchanged and + // `.shape` still enumerates twelve keys. `type` is an open widget hint, so + // this reads ONE member of it — the member whose whole meaning is "resolve + // records from another object", which cannot be done without naming one. + if (field.type !== 'lookup') return; + if (typeof field.reference === 'string' && field.reference.trim() !== '') return; + ctx.addIssue({ + code: 'custom', + path: ['reference'], + message: SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED, + }); })); export type ScreenFieldConfig = z.input; diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index 2d82047493..1723f567cb 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/packages/spec/src/contracts/automation-service.ts @@ -161,6 +161,43 @@ export interface ScreenFieldSpec { options?: Array<{ value: unknown; label: string }>; defaultValue?: unknown; placeholder?: string; + /** + * Numeric bound pair the client applies to a numeric input (#17306), + * spelled as the object field spells it (`FieldSchema.min` / `.max`). + * + * Unlike {@link visibleWhen}, these are NOT client-only: the server + * re-checks them when the run resumes (`validateScreenInputs`), so a bound + * holds against a caller that skips the dialog — with no "if the value + * happens to be a number" qualifier, because on a `type: 'number'` field a + * present non-number is refused outright (`invalid_type`) rather than + * passing the bound it cannot be compared against. ⛔ Not coerced. The + * client still applies them so the user is stopped at the input rather + * than at Submit. + */ + min?: number; + max?: number; + /** + * Help text rendered under the input (#17306) — `FieldSchema`'s spelling. + * Distinct from {@link placeholder}, which the browser clears the moment a + * value is typed, and so cannot carry a constraint the user must keep + * reading. + */ + inlineHelpText?: string; + /** + * Object whose records a `type: 'lookup'` field picks from (#17306) — + * `FieldSchema.reference`'s spelling. + * + * Optional HERE and required on the authoring side: `ScreenFieldConfigSchema` + * refuses a `type: 'lookup'` field that declares no `reference` (maintainer + * ruling A′, 2026-09-13), so a screen reaching a client through a parsed + * flow always carries it. This is the WIRE shape, and it stays optional for + * the reason every key on it is: it also describes a `ScreenSpec` rehydrated + * from a run SUSPENDED before the upgrade, which was stored under the old + * accept set. A client that meets one has no target to resolve a picker + * against and falls back to a plain input — the pre-#17306 behaviour, kept + * reachable on purpose rather than turned into a client-side crash. + */ + reference?: string; /** * Conditional-visibility predicate (ADR-0089's canonical key), evaluated by * the CLIENT against the screen's live collected values — not by the server, diff --git a/packages/spec/src/migrations/entries/semantic/18.screen-field-lookup-reference-required.ts b/packages/spec/src/migrations/entries/semantic/18.screen-field-lookup-reference-required.ts new file mode 100644 index 0000000000..dfb2dda74a --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.screen-field-lookup-reference-required.ts @@ -0,0 +1,63 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// A `type: 'lookup'` screen field must now name its target object. Registered as +// SEMANTIC and never as a D2 conversion because the remedy is a value the stored +// metadata does not contain: a bare lookup says which field it is, never which +// object it meant, so any transform would have to invent one. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'screen-field-lookup-reference-required', + surface: + "The `reference` key of a `type: 'lookup'` field on a `screen` node — " + + '`flows[].nodes[].config.fields[]` where the node `type` is `screen` and the field ' + + "`type` is `lookup` (`ScreenFieldConfigSchema`). Nothing is renamed, retired or " + + 're-typed and the key set does not move: `reference` was already declared and ' + + 'already optional in the shape. What narrows is the ACCEPT SET for one value of the ' + + "sibling `type` — a `lookup` field with no `reference`, or with a blank one, parsed " + + 'before this major and is refused now. Every other widget hint is untouched, and a ' + + '`lookup` field that already names its target parses byte-identically.', + replacement: + "Name the object whose records the picker offers, beside the type: " + + "`{ name: 'resolved_by_article', type: 'lookup', reference: 'crm_knowledge_article' }`. " + + 'The value is an object NAME (the canonical id — same string `FieldSchema.reference` ' + + 'carries), not a label and not a record id. ⚠️ There is deliberately no default and no ' + + 'inference: a picker pointed at the wrong object is worse than one that refuses to ' + + 'load, because it offers a human a plausible list of the wrong records and the flow ' + + 'stores the id it is given. Where the field genuinely has no target object — the ' + + 'author was using `lookup` to mean "type an id here" — the fix is the other ' + + "direction: change `type` to `'text'`, which is what that field actually was, and " + + 'keep the prose that asked for an id in `inlineHelpText`.', + reason: + 'Maintainer ruling A′, 2026-09-13 (decision batch #130 item 1), verbatim, ' + + 'untranslated: 「同意」. ADR-0078 forbids metadata that parses, carries no marking and ' + + 'does nothing — and its own worked example of that state is a `lookup` with no ' + + '`reference`: the field renders a picker, the picker has no object to query, and ' + + 'nothing anywhere says so. The key shipped OPTIONAL on this surface one release ' + + 'earlier, on the argument that flows declaring a bare `lookup` already exist; the ' + + 'ruling reversed that, holding that a degraded shape which ships is not a reason to ' + + 'bend the contract to it. ⛔ NOT losslessly convertible, and the reason is the same ' + + 'one `schedule-flow-acting-organization-required` gives: the remedy is a value the ' + + 'artifact does not contain. A bare lookup records the field name and nothing about ' + + 'its intended object, so `objectstack migrate meta` can identify every site but can ' + + 'answer none of them — and a conversion that guessed (the first object with a ' + + 'matching-looking name, the flow\'s trigger object) would write an authoritative ' + + 'wrong answer into metadata a human then trusts. Registered under ADR-0087 D3 rather ' + + 'than left silent because the change DOES carry a prescription a human can execute, ' + + 'which is what D3 says a structured TODO is for.', + acceptanceCriteria: + "Every `type: 'lookup'` field on every `screen` node in the stack declares a non-empty " + + '`reference`, and the stack parses: `ScreenFieldConfigSchema` refuses the bare form ' + + 'with a message addressed to `reference` ' + + '(`SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED`), so a full metadata parse — `os lint`, or ' + + 'any publish — reports one issue per unfixed site and names the FLOW and the FIELD in ' + + 'its path. Work the list to empty rather than sampling it: a flow whose screen never ' + + 'reaches that node in testing is refused at publish just the same. For each site, ' + + 'answer which object the picker was meant to offer — the declaration is the answer, ' + + "and where there is no such object the field was never a lookup (retype it `'text'`). " + + '⚠️ Runs SUSPENDED at a screen before the upgrade rehydrate their `ScreenSpec` from ' + + 'stored context, so an in-flight run parked on an unfixed screen carries the old ' + + 'shape: drain or re-drive those rather than assuming the fix reaches them ' + + 'retroactively.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 29109ec367..41781d84df 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -9792,6 +9792,60 @@ const step18: MigrationStep = { + 'tables in deployed databases are left in place untouched, by ruling — ' + 'no backfill, no reaper, no migrate command.', }, + { + id: 'screen-field-lookup-reference-required', + surface: + "The `reference` key of a `type: 'lookup'` field on a `screen` node — " + + '`flows[].nodes[].config.fields[]` where the node `type` is `screen` and the field ' + + "`type` is `lookup` (`ScreenFieldConfigSchema`). Nothing is renamed, retired or " + + 're-typed and the key set does not move: `reference` was already declared and ' + + 'already optional in the shape. What narrows is the ACCEPT SET for one value of the ' + + "sibling `type` — a `lookup` field with no `reference`, or with a blank one, parsed " + + 'before this major and is refused now. Every other widget hint is untouched, and a ' + + '`lookup` field that already names its target parses byte-identically.', + replacement: + "Name the object whose records the picker offers, beside the type: " + + "`{ name: 'resolved_by_article', type: 'lookup', reference: 'crm_knowledge_article' }`. " + + 'The value is an object NAME (the canonical id — same string `FieldSchema.reference` ' + + 'carries), not a label and not a record id. ⚠️ There is deliberately no default and no ' + + 'inference: a picker pointed at the wrong object is worse than one that refuses to ' + + 'load, because it offers a human a plausible list of the wrong records and the flow ' + + 'stores the id it is given. Where the field genuinely has no target object — the ' + + 'author was using `lookup` to mean "type an id here" — the fix is the other ' + + "direction: change `type` to `'text'`, which is what that field actually was, and " + + 'keep the prose that asked for an id in `inlineHelpText`.', + reason: + 'Maintainer ruling A′, 2026-09-13 (decision batch #130 item 1), verbatim, ' + + 'untranslated: 「同意」. ADR-0078 forbids metadata that parses, carries no marking and ' + + 'does nothing — and its own worked example of that state is a `lookup` with no ' + + '`reference`: the field renders a picker, the picker has no object to query, and ' + + 'nothing anywhere says so. The key shipped OPTIONAL on this surface one release ' + + 'earlier, on the argument that flows declaring a bare `lookup` already exist; the ' + + 'ruling reversed that, holding that a degraded shape which ships is not a reason to ' + + 'bend the contract to it. ⛔ NOT losslessly convertible, and the reason is the same ' + + 'one `schedule-flow-acting-organization-required` gives: the remedy is a value the ' + + 'artifact does not contain. A bare lookup records the field name and nothing about ' + + 'its intended object, so `objectstack migrate meta` can identify every site but can ' + + 'answer none of them — and a conversion that guessed (the first object with a ' + + 'matching-looking name, the flow\'s trigger object) would write an authoritative ' + + 'wrong answer into metadata a human then trusts. Registered under ADR-0087 D3 rather ' + + 'than left silent because the change DOES carry a prescription a human can execute, ' + + 'which is what D3 says a structured TODO is for.', + acceptanceCriteria: + "Every `type: 'lookup'` field on every `screen` node in the stack declares a non-empty " + + '`reference`, and the stack parses: `ScreenFieldConfigSchema` refuses the bare form ' + + 'with a message addressed to `reference` ' + + '(`SCREEN_FIELD_LOOKUP_REFERENCE_REQUIRED`), so a full metadata parse — `os lint`, or ' + + 'any publish — reports one issue per unfixed site and names the FLOW and the FIELD in ' + + 'its path. Work the list to empty rather than sampling it: a flow whose screen never ' + + 'reaches that node in testing is refused at publish just the same. For each site, ' + + 'answer which object the picker was meant to offer — the declaration is the answer, ' + + "and where there is no such object the field was never a lookup (retype it `'text'`). " + + '⚠️ Runs SUSPENDED at a screen before the upgrade rehydrate their `ScreenSpec` from ' + + 'stored context, so an in-flight run parked on an unfixed screen carries the old ' + + 'shape: drain or re-drive those rather than assuming the fix reaches them ' + + 'retroactively.', + }, { id: 'send-template-input-org-retired', surface: 'contracts.emailService.sendTemplate input.org', diff --git a/packages/spec/src/system/i18n-resolver.ts b/packages/spec/src/system/i18n-resolver.ts index edd8dfb635..39d432d69f 100644 --- a/packages/spec/src/system/i18n-resolver.ts +++ b/packages/spec/src/system/i18n-resolver.ts @@ -3357,10 +3357,17 @@ export type FlowScreenCopyKey = typeof FLOW_SCREEN_COPY_KEYS[number]; /** * The copy keys `flows..screens..fields.` carries — * the per-FIELD face of {@link FLOW_SCREEN_COPY_KEYS}, measured against - * `ScreenFieldConfigSchema`. `help` is deliberately absent: the screen field - * declares nothing help-shaped at all, so a `help` key would parse clean and - * translate nothing (the ADR-0078 shape #6080 kept out of the page-component - * face); `options` is absent because `ScreenFieldConfig.options[].value` is + * `ScreenFieldConfigSchema`. + * + * `help` is deliberately absent. ⚠️ Not for its original reason any more: the + * screen field used to declare nothing help-shaped, so a `help` key would have + * parsed clean and translated nothing (the ADR-0078 shape #6080 kept out of the + * page-component face). #17306 gave it `inlineHelpText`, so the string exists — + * what does not exist is a key on THIS face for it, and growing the face is a + * ruled step against the #7646 enumeration, never a resolver-side accretion. + * The exclusion therefore stands with the same outcome and a different reason. + * + * `options` is absent because `ScreenFieldConfig.options[].value` is * unconstrained, so a value-keyed map cannot address the labels. Both are * refused by name with guidance at the schema. */ diff --git a/packages/spec/src/system/translation.test.ts b/packages/spec/src/system/translation.test.ts index 95213cbdc7..f64a71d993 100644 --- a/packages/spec/src/system/translation.test.ts +++ b/packages/spec/src/system/translation.test.ts @@ -1135,13 +1135,17 @@ describe('translation unknown-key strictness (#4001)', () => { .toContain('`title` → `label`'); }); - it('refuses `help` on a screen field rather than declaring a key the field has not got', () => { - // The report proposed label/placeholder/help. `ScreenFieldConfigSchema` - // declares nothing help-shaped, so `help` would parse clean and translate - // nothing (ADR-0078) — and unlike #6080's page-component `help` there is - // no honest key to alias it to, since `placeholder` is the in-input hint - // and not help text. It is `guidance` instead. + it('refuses `help` on a screen field, and says the string exists but the key does not', () => { + // The report proposed label/placeholder/help. `help` is still refused — + // but ⚠️ its reason changed with #17306 and this pin changed with it. + // The old reason was that the field declared nothing help-shaped; it now + // declares `inlineHelpText` (the object field's spelling), so the copy is + // real and only THIS face's key for it is missing. The refusal must not + // keep telling an author the field has no help copy when it has. const declared = Object.keys((ScreenFieldConfigSchema as unknown as z.ZodObject).shape); + expect(declared).toContain('inlineHelpText'); + // The bare spellings stay undeclared on the schema — `inlineHelpText` is + // the one landing key, so the translation face has exactly one candidate. expect(declared).not.toContain('help'); expect(declared).not.toContain('helpText'); expect(declared).toContain('label'); @@ -1149,9 +1153,14 @@ describe('translation unknown-key strictness (#4001)', () => { const message = parse({ lead_conversion: { screens: { s1: { fields: { f: { help: 'x' } } } } } }) .error?.issues.find((i) => i.code === 'unrecognized_keys')?.message ?? ''; - expect(message).toContain('a screen field declares no help/hint copy'); + expect(message).toContain('would translate nothing'); + expect(message).toContain('inlineHelpText'); // …and it must not be re-pointed at `placeholder`, which means something else. expect(message).not.toContain('`help` → `placeholder`'); + // The card that moved this reason is named in the code comment above the + // string, never IN the string: this text is printed AT the author, who + // has no tracker, so `#NNNN` resolves to nothing (check:doc-authoring). + expect(message).not.toMatch(/#\d{3,5}\b/); }); it('says why select-option labels are not translatable here', () => { diff --git a/packages/spec/src/system/translation.zod.ts b/packages/spec/src/system/translation.zod.ts index 92e76a54e7..a6bea0239b 100644 --- a/packages/spec/src/system/translation.zod.ts +++ b/packages/spec/src/system/translation.zod.ts @@ -601,15 +601,26 @@ const TRANSLATION_KEY_GUIDANCE: Record.dimensions.` and @@ -1055,13 +1066,17 @@ const translationDataShape = () => ({ * report.** Two of the three per-field keys the issue proposed are real * (`label`, `placeholder`); `help` is not: * - * - **`help` is not here** — `ScreenFieldConfigSchema` declares - * `name`/`label`/`type`/`required`/`options`/`defaultValue`/`placeholder`/`visibleWhen` - * and nothing help-shaped at all. Declaring it would parse clean and - * translate nothing, the ADR-0078 shape #6080 removed from the page - * component face for exactly this reason. It is `guidance` instead, so an - * author who reaches for it is told the field has no such copy rather than - * sent to a neighbouring key that means something else. + * - **`help` is not here.** Originally because `ScreenFieldConfigSchema` + * declared nothing help-shaped at all, so the key would have parsed clean + * and translated nothing — the ADR-0078 shape #6080 removed from the page + * component face for exactly this reason. ⚠️ That premise expired with + * #17306, which gave the screen field `inlineHelpText` (the object field's + * own spelling). The copy now exists; this face's key for it does not, and + * growing the face is a ruled step against the #7646 enumeration rather + * than a resolver-side accretion. So the exclusion stands and the outcome + * is unchanged — a `help` entry still translates nothing — but it is now a + * NOT-YET, and the guidance says so rather than telling an author the field + * has no help copy when it has. * * ⛔ **Runner chrome is NOT here** — the Cancel/Submit buttons the wizard * draws around the author's screen belong to the console's own message