diff --git a/.changeset/25302-get-record-limit-positive-integer.md b/.changeset/25302-get-record-limit-positive-integer.md new file mode 100644 index 00000000000..ba6a6be191d --- /dev/null +++ b/.changeset/25302-get-record-limit-positive-integer.md @@ -0,0 +1,24 @@ +--- +'@objectstack/spec': minor +--- + +fix(spec)!: a flow `get_record` node's `limit` is declared a positive integer, so a fraction, `0` or a negative is refused at every door that parses a flow + +Clause-②: no (narrowing) +Metadata-protocol: narrowed + + + +**BREAKING** (a value the parse accepted is now refused), shipped as `minor` under the 18.x convention for breaking changes, with no retirement stage. + +The `get_record` executor reads around the Data API's page-window normalizer: with `limit` above 1 it calls `data.find` with that LIMIT as written, and otherwise it calls `findOne`. The spec declared `limit` as any number, so a flow could declare a window the runtime did not honour as written. + +- **FROM:** `GetRecordConfigSchema.limit` was `z.number()`. `limit: 2.5` parsed and reached the driver as `LIMIT 2.5`; `limit: 0`, `limit: -1` and `limit: 0.5` parsed and silently read one record through `findOne`. +- **TO:** `limit` is `z.number().int().positive()`. Every door that parses a flow refuses any other number at `nodes.N.config.limit` with the code `node-config-refused-by-contract`: `FlowSchema.parse`, `defineStack` (`STACK_SCHEMA_INVALID`, 422), `objectstack validate` and `objectstack compile`, the metadata save door, `AutomationEngine.registerFlow`, and an artifact's parse. The generated JSON Schema declares `type: integer` with an exclusive minimum of 0. +- **Fix:** write the count as a whole number of at least 1. `limit: 1` (or no `limit`) reads one record as `record`; a larger whole number reads a `records` list of at most that many. +- **Unchanged:** every positive whole `limit`, and a node with no `limit`. A string `limit` (`'10'`, `'{page.size}'`) was already refused as the wrong type. +- **Not the Data API's page window.** `QueryPageWindowSchema` admits `0`, which asks a list door for the total alone. A `get_record` node always reads at least one row, so its bound is its own. + +Data at rest: a flow already stored with such a `limit` is refused whole when it registers. At boot it is skipped with a `warn` naming it and its trigger is not armed, while the flows beside it register. `objectstack migrate meta` reports it under `flow-builtin-node-config-values-refused`. No conversion rewrites it, because only the author knows the count they meant. + +No producer writes such a value: every `get_record` `limit` in this repository's examples and tests, and in HotCRM, is a positive whole number. diff --git a/content/docs/references/automation/builtin-node-config.mdx b/content/docs/references/automation/builtin-node-config.mdx index 712bc983126..64f1b558148 100644 --- a/content/docs/references/automation/builtin-node-config.mdx +++ b/content/docs/references/automation/builtin-node-config.mdx @@ -233,7 +233,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr | **objectName** | `string` | ✅ | Object to query | | **filter** | `Record` | optional | Field/value pairs to match: each operand — a field's value, an operator's comparand (`{"$ne": null}`), a list comparand's element — is a CEL value envelope `{ dialect: 'cel', source }` evaluated before the query (one that evaluates to null refuses the node), a query placeholder such as `{current_user_id}` or `{today}` the query engine resolves, or a literal; a flow `{…}` template token is refused | | **fields** | `string[]` | optional | Field projection — only these fields are read (default: all) | -| **limit** | `number` | optional | Max records to return; >1 switches to a multi-record query | +| **limit** | `integer` | optional | Max records to return — a positive integer; >1 switches to a multi-record query | | **outputVariable** | `string` | optional | Flow variable the result is bound to — a name without a leading `$` (the `$` names are the flow engine's own), read as `{{ name }}` | diff --git a/packages/spec/src/automation/builtin-node-config.zod.ts b/packages/spec/src/automation/builtin-node-config.zod.ts index 83e9fea5026..a900d3e77c5 100644 --- a/packages/spec/src/automation/builtin-node-config.zod.ts +++ b/packages/spec/src/automation/builtin-node-config.zod.ts @@ -609,8 +609,9 @@ const FILTER_OPERAND_DESCRIPTION = * `filter` values may carry operator objects (`{"$ne": null}`), and each * operand is a CEL value envelope, a query placeholder or a literal * ({@link flowFilterSlotSchema}); an envelope that evaluates to nothing - * REFUSES the node rather than widening the query (#3810). `limit > 1` selects - * `find` (a `records` list); otherwise `findOne` (a single `record`). + * REFUSES the node rather than widening the query (#3810). `limit` is a + * positive integer: `limit > 1` selects `find` (a `records` list); otherwise + * `findOne` (a single `record`). */ export const GetRecordConfigSchema = lazySchema(() => strictObject({ surface: 'this get_record node config', @@ -624,9 +625,16 @@ export const GetRecordConfigSchema = lazySchema(() => strictObject({ /** Field projection — only these fields are read (default: all). */ fields: z.array(z.string()).optional() .describe('Field projection — only these fields are read (default: all)'), - /** Max records; >1 returns a `records` list, otherwise a single `record`. */ - limit: z.number().optional() - .describe('Max records to return; >1 switches to a multi-record query'), + /** + * Max records — a positive integer; >1 returns a `records` list, otherwise a + * single `record`. The executor hands it to `data.find` as the query's LIMIT + * as written, outside the Data API's page-window normalizer, so a fraction or + * a count below 1 is refused here, at every door that parses a flow (#25302). + * Not the Data API's `QueryPageWindowSchema`: `0` is a window there (the + * total alone), while this node always reads at least one row. + */ + limit: z.number().int().positive().optional() + .describe('Max records to return — a positive integer; >1 switches to a multi-record query'), /** Flow variable the result is bound to — never a `$` name: those are the engine's (#22502). */ outputVariable: flowBoundVariableNameSchema('outputVariable').optional() .describe('Flow variable the result is bound to — a name without a leading `$` (the `$` names are the flow engine\'s own), read as `{{ name }}`'), diff --git a/packages/spec/src/automation/flow-builtin-node-config-values.test.ts b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts index 542bb7e2956..5470205f902 100644 --- a/packages/spec/src/automation/flow-builtin-node-config-values.test.ts +++ b/packages/spec/src/automation/flow-builtin-node-config-values.test.ts @@ -139,6 +139,10 @@ describe('one refusal per judged builtin, at each typed key', () => { // [type, the edit onto its VALID config, the key the refusal names] const PROBES: ReadonlyArray = [ ['get_record', { limit: '10' }, 'limit'], + // [#25302] The executor hands `limit` to the driver as the LIMIT as written. + ['get_record', { limit: 2.5 }, 'limit'], + ['get_record', { limit: 0 }, 'limit'], + ['get_record', { limit: -1 }, 'limit'], ['get_record', { fields: 'name' }, 'fields'], ['create_record', { objectName: 5 }, 'objectName'], ['update_record', { multi: 'true' }, 'multi'], @@ -336,6 +340,64 @@ describe('[#23260] a value carrying a `{token}` is judged for its type, like any }); }); +/** + * [#25302] `get_record`'s `limit` is a positive integer. The executor does not + * read through `findData`, so the Data API's page-window normalizer never + * judges it: `limit > 1` is handed to `data.find` as the LIMIT as written + * (`2.5` included), and anything else (`0`, `-1`, `0.5`) silently became a + * single-record `findOne`. The contract now refuses the value at every door. + */ +describe('[#25302] get_record limit is a positive integer', () => { + it('a fraction is refused at nodes.1.config.limit, in the contract\'s words', () => { + const config = { ...VALID.get_record, limit: 2.5 }; + const issues = issuesOf(flowWith('get_record', config)); + expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([{ code: 'custom', path: 'nodes.1.config.limit' }]); + expect(contractSentence('get_record', config, 'limit')).not.toBe(''); + expect(issues[0]!.message).toContain(contractSentence('get_record', config, 'limit')); + }); + + it.each([[2.5], [0], [-1], [0.5], [Number.MAX_SAFE_INTEGER + 2]])( + 'defineStack refuses limit %s in its ADR-0112 envelope, at flows.0.nodes.1.config.limit', + (limit) => { + let refusal: { code?: unknown; status?: unknown; issues?: Array<{ path: unknown[]; code: string }> } | undefined; + try { + defineStack({ + manifest: { id: 'com.example.limit', name: 'limit', version: '1.0.0', type: 'app', namespace: 'lim' }, + objects: [{ name: 'lim_account', label: 'Account', fields: { name: { type: 'text', label: 'Name' } } }], + flows: [flowWith('get_record', { objectName: 'lim_account', limit }, 'lim_probe')], + } as never); + } catch (e) { + refusal = e as typeof refusal; + } + expect(refusal, 'defineStack must refuse the flow').toBeDefined(); + expect({ code: refusal!.code, status: refusal!.status }).toEqual({ code: 'STACK_SCHEMA_INVALID', status: 422 }); + expect(refusal!.issues!.map((i) => ({ path: i.path.join('.'), code: i.code }))).toEqual([ + { path: 'flows.0.nodes.1.config.limit', code: 'custom' }, + ]); + }, + ); + + it('the registered `flow` type schema — what the metadata save door validates against — refuses it', () => { + const schema = getMetadataTypeSchema('flow') as unknown as typeof FlowSchema; + const r = schema.safeParse(flowWith('get_record', { ...VALID.get_record, limit: 0 })); + expect(r.success).toBe(false); + expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['nodes.1.config.limit']); + }); + + it.each([[1], [2], [200]])('CONTROL: limit %s parses, and the judge says nothing', (limit) => { + const config = { ...VALID.get_record, limit }; + expect(flowNodeConfigRefusals('get_record', config)).toEqual([]); + const r = FlowSchema.safeParse(flowWith('get_record', config)); + expect(r.success, JSON.stringify(r.error?.issues ?? [])).toBe(true); + }); + + it('CONTROL: a node with no limit parses unchanged', () => { + const { limit: _omitted, ...config } = VALID.get_record; + expect(flowNodeConfigRefusals('get_record', config)).toEqual([]); + expect(FlowSchema.safeParse(flowWith('get_record', config)).success).toBe(true); + }); +}); + describe('what stays accepted (lit controls)', () => { it('CONTROL: a valid node of every builtin type parses, and the judge says nothing', () => { expect(Object.keys(VALID).sort()).toEqual([...getBuiltinNodeConfigContracts().keys()].sort()); diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts index 6f29de9f8f6..7d940b92037 100644 --- a/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts @@ -10,6 +10,10 @@ import type { SemanticMigration } from '../../types.js'; // narrows a flow's accept set; no key is removed, so there is no tombstone and // no RETIRED_KEYS_BY_MAJOR row. There is no D2 conversion either: the platform // cannot know the value the author meant, and the runtime never ran such a node. +// (#25302) The one exception is `get_record`'s `limit` range: a fraction or a +// count below 1 did run, as the driver's LIMIT as written (`2.5`) or as a +// single-record `findOne` (`0`, `-1`), so its refusal changes what such a node +// did rather than how it fails, and still no conversion can know the count. // // No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it // inside a code span and a table cell. @@ -21,7 +25,7 @@ export const entry: SemanticMigration = { + 'contract refuses — a value of the wrong type (create_record outputVariable 42, a screen field min ' + 'written as the string 1, get_record limit as a string, update_record multi as a string), a value ' + 'outside the declared set or range (notify severity loud, screen mode view, loop maxIterations 0, ' - + 'try_catch retry.maxRetries above 10), an empty script function or subflow flowName, or a rule ' + + 'try_catch retry.maxRetries above 10, get_record limit 2.5 or 0 where a positive integer is declared), an empty script function or subflow flowName, or a rule ' + 'finding on present keys (a notify template beside an inline title). A value carrying a token in ' + 'braces is judged like any other (get_record limit written as a token, refused as the wrong type), ' + 'and at a key its executor reads as written (a notify sourceObject, actorId or payload; an http ' @@ -36,7 +40,8 @@ export const entry: SemanticMigration = { + 'string (`outputVariable: \'taskId\'`), a number where it wants a number (`min: 1`, `limit: 10`, ' + '`maxIterations: 5`, `timeoutMs: 5000`), a boolean where it wants a boolean (`multi: true`, ' + '`durable: true`), one of the declared values (`severity: \'warning\'`, `mode: \'edit\'`), or a value ' - + 'inside the declared range. A number or boolean slot takes a LITERAL only: every builtin executor ' + + 'inside the declared range (a `get_record` `limit` is a positive integer: `limit: 1` reads one record, ' + + 'a larger whole number a list of at most that many). A number or boolean slot takes a LITERAL only: every builtin executor ' + 'parses the config as authored, so a `{token}` template there (`limit: \'{page.size}\'`, ' + '`maxIterations: \'{cap}\'`) is refused at the build doors as the wrong type, as every run refused ' + 'it — and at an `http` node\'s `durable` and `timeoutMs`, which `http` once interpolated before it ' @@ -72,7 +77,10 @@ export const entry: SemanticMigration = { + 'refused: registered from the metadata registry or `sys_metadata` at boot it is skipped with a ' + '`warn` naming it, its trigger not armed, while the flows beside it register; a ' + '`defineStack({ flows })` source throws `StackSchemaInvalidError` for the whole stack; an artifact ' - + 'file is refused whole at load. ADR-0087, ADR-0031.', + + 'file is refused whole at load. One value in this set used to run rather than fail: a `get_record` ' + + '`limit` that is not a positive integer. The executor reads around the Data API\'s page-window ' + + 'normalizer, so `limit: 2.5` reached the driver as its LIMIT as written, and `0` or `-1` silently read ' + + 'one record through `findOne`. ADR-0087, ADR-0031.', acceptanceCriteria: 'Run `objectstack validate` over every stack authored in config files, and boot every deployed ' + 'stack. Each refusal names the node and the key: `FlowSchema.parse` anchors a `custom` issue at '