Skip to content

Commit 303aef0

Browse files
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 (#25357)
<!-- claude-projects-attribution: {"github_login":"os-tesla"} --> _Requested by **Tesla** · [project thread](https://claude.ai/code/project/chan_01AXsQkLFKRdQCqQ3RTr6Jao?thread=cmsg_01AXsQkLFKRdQCqQ3RTr6JaoBtFLBZ6RGauULKM9CboqUv)_ Fixes #25302 Clause-②: no (narrowing) Metadata-protocol: narrowed ## What changes `GetRecordConfigSchema.limit` (`packages/spec/src/automation/builtin-node-config.zod.ts`) goes from `z.number()` to `z.number().int().positive()`. The `get_record` executor (`packages/services/service-automation/src/builtin/crud-nodes.ts`) parses its config against this contract, then calls `data.find` with `limit` as written when `limit > 1`, and `findOne` otherwise. It does not go through `findData`, so the Data API's page-window normalizer never judges it. Before this change `limit: 2.5` reached the driver as its LIMIT, and `0`, `-1` or `0.5` silently read one record. The executor contract is the one judge every flow door shares (`flowNodeConfigRefusals`, the value half). Narrowing it refuses the value at `nodes.N.config.limit` with `node-config-refused-by-contract` at `FlowSchema.parse`, `defineStack` (`STACK_SCHEMA_INVALID`, 422), `objectstack validate` / `compile`, the registered `flow` type schema the metadata save door validates against, `AutomationEngine.registerFlow`, and an artifact's parse. No consumer code changes. - **Why not reuse the PR #25300 exports.** `QueryPageWindowSchema` is `int().nonnegative()`, and `0` means "the total alone" at a list door. A `get_record` node always reads at least one row, so `0` there is not a window. The bound is its own, and the TSDoc says why. - **ADR-0087 disposition:** `not-required (already-registered flow-builtin-node-config-values-refused)`, amended in this diff. That D3 entry already covers "a value outside the declared set or range" for builtin node configs. Its surface, replacement and reason now name `get_record` `limit`. The reason also says the one difference: such a node used to run rather than fail. No D2 conversion, because only the author knows the count they meant. - **Changeset:** `.changeset/25302-get-record-limit-positive-integer.md`, `@objectstack/spec` `minor`, with FROM → TO and the one-line fix. - **Generated:** `content/docs/references/automation/builtin-node-config.mdx` (`check:generated --fix`, 1 of 14 artifacts stale: `check:docs`). The JSON Schema (gitignored) now reads `type: integer`, `exclusiveMinimum: 0`. ## Producers measured - **This repo** at `97967a9`: 70 `get_record` nodes in `*.ts`. Every `limit` within 14 lines of one is a positive whole number (200 ×4, 5 ×5, 50 ×2, 10 ×2). The one other value is the string `'{page.size}'` in `canonicalize-stored-flow.test.ts`, which is already refused as the wrong type and still throws `/limit/`. Query: `git grep -n -A14 -E "type: ?['\"]get_record['\"]"` piped to a grep for `limit:`. - **HotCRM** at `4166f21` (current `main`): 42 `get_record` nodes. 12 `limit`s, all positive whole numbers (500 ×7, 1000 ×2, 1 ×1, and `BILLING_HANDOFF_LINE_ITEM_LIMIT` = 200 ×2). ## Tests At head `d972d18` (the spec diff is unchanged since `e92f351`; the merge brought plugin-security/plugin-sharing only): - `flow-builtin-node-config-values.test.ts`: probes `limit` 2.5 / 0 / -1 at the judge, and a new `get_record limit is a positive integer` block. That block covers a fraction at `FlowSchema` in the contract's words; `defineStack` asserting `code` + `status` (`STACK_SCHEMA_INVALID` / 422) and the path, for 2.5, 0, -1, 0.5 and an unsafe integer; and the save-door schema. Controls: `limit` 1, 2 and 200 parse, and no `limit` parses. - `@objectstack/spec` test: 709/709 files, 21566 passed. typecheck: exit 0. - Consumers (filter direction: downstream readers of the schema): `@objectstack/service-automation` 198/198 files, 2618 tests; `@objectstack/lint` 153/153, 7076; `@objectstack/metadata-protocol` 259 passed + 3 skipped, 21416 passed. typecheck exit 0 for all three. No fixture needed triage, because no fixture carries a non-positive or fractional `limit`. - **Ablation** (`scripts/ablation-replace.mjs`, mutation `int().positive()` removed): the test file went 10 failed / 59 passed, which is exactly the 10 refusal cases. The controls stayed green. Restored: blob equals HEAD `a900d3e7` and `git diff HEAD` is empty. Spec's own tests import `src`, so no build leg applies. - Dogfood: see the report comment on the card (it runs on the final head). ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 113 families for this diff. Each was run locally with its exit code captured before any pipe. 112 exited 0. `pnpm check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET: it reads the dist of about 40 packages this diff does not build), so it is NOT MEASURED locally and left to CI's `Lint & Repo Gates`. `check:skill-examples` first exited 3 (no client dist), then exited 0 after building `@objectstack/client` and `client-react`. Three gates first exited 3 on a shallow clone or a missing lint dist, then exited 0 after `git fetch --unshallow` and building the lint closure. ## Acceptance notes - The executor's own form descriptor in `crud-nodes.ts` declares `limit: { type: 'integer', title: 'Limit' }` with no `minimum`. So a designer form built from it offers `0`, which the save door now refuses with the contract's words. That is outside this card's file surface. Noted, not filed: declared ≠ enforced does not apply, because the refusal is loud at save. - `if (limit && limit > 1)` in the executor is now exact: `limit` is either absent or a whole number of at least 1. --- _Generated by [Claude Code](https://claude.ai/code/session_013XF6vcPvtFkNY5BmaMGzcd)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b3034ee commit 303aef0

5 files changed

Lines changed: 111 additions & 9 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
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
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: narrowed
9+
10+
<!-- adr-0087: not-required (already-registered flow-builtin-node-config-values-refused) registered before this change, amended in this diff: the entry's surface, replacement and reason now name a get_record limit that is not a positive integer, and the stored-row listing reports such a node under that id -->
11+
12+
**BREAKING** (a value the parse accepted is now refused), shipped as `minor` under the 18.x convention for breaking changes, with no retirement stage.
13+
14+
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.
15+
16+
- **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`.
17+
- **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.
18+
- **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.
19+
- **Unchanged:** every positive whole `limit`, and a node with no `limit`. A string `limit` (`'10'`, `'{page.size}'`) was already refused as the wrong type.
20+
- **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.
21+
22+
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.
23+
24+
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.

‎content/docs/references/automation/builtin-node-config.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ A value: a CEL value envelope `{ dialect: 'cel', source }` evaluated by the expr
233233
| **objectName** | `string` | ✅ | Object to query |
234234
| **filter** | `Record<string, any>` | 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 |
235235
| **fields** | `string[]` | optional | Field projection — only these fields are read (default: all) |
236-
| **limit** | `number` | optional | Max records to return; >1 switches to a multi-record query |
236+
| **limit** | `integer` | optional | Max records to return — a positive integer; >1 switches to a multi-record query |
237237
| **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 }}` |
238238

239239

‎packages/spec/src/automation/builtin-node-config.zod.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -609,8 +609,9 @@ const FILTER_OPERAND_DESCRIPTION =
609609
* `filter` values may carry operator objects (`{"$ne": null}`), and each
610610
* operand is a CEL value envelope, a query placeholder or a literal
611611
* ({@link flowFilterSlotSchema}); an envelope that evaluates to nothing
612-
* REFUSES the node rather than widening the query (#3810). `limit > 1` selects
613-
* `find` (a `records` list); otherwise `findOne` (a single `record`).
612+
* REFUSES the node rather than widening the query (#3810). `limit` is a
613+
* positive integer: `limit > 1` selects `find` (a `records` list); otherwise
614+
* `findOne` (a single `record`).
614615
*/
615616
export const GetRecordConfigSchema = lazySchema(() => strictObject({
616617
surface: 'this get_record node config',
@@ -624,9 +625,16 @@ export const GetRecordConfigSchema = lazySchema(() => strictObject({
624625
/** Field projection — only these fields are read (default: all). */
625626
fields: z.array(z.string()).optional()
626627
.describe('Field projection — only these fields are read (default: all)'),
627-
/** Max records; >1 returns a `records` list, otherwise a single `record`. */
628-
limit: z.number().optional()
629-
.describe('Max records to return; >1 switches to a multi-record query'),
628+
/**
629+
* Max records — a positive integer; >1 returns a `records` list, otherwise a
630+
* single `record`. The executor hands it to `data.find` as the query's LIMIT
631+
* as written, outside the Data API's page-window normalizer, so a fraction or
632+
* a count below 1 is refused here, at every door that parses a flow (#25302).
633+
* Not the Data API's `QueryPageWindowSchema`: `0` is a window there (the
634+
* total alone), while this node always reads at least one row.
635+
*/
636+
limit: z.number().int().positive().optional()
637+
.describe('Max records to return — a positive integer; >1 switches to a multi-record query'),
630638
/** Flow variable the result is bound to — never a `$` name: those are the engine's (#22502). */
631639
outputVariable: flowBoundVariableNameSchema('outputVariable').optional()
632640
.describe('Flow variable the result is bound to — a name without a leading `$` (the `$` names are the flow engine\'s own), read as `{{ name }}`'),

‎packages/spec/src/automation/flow-builtin-node-config-values.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ describe('one refusal per judged builtin, at each typed key', () => {
139139
// [type, the edit onto its VALID config, the key the refusal names]
140140
const PROBES: ReadonlyArray<readonly [string, Config, string]> = [
141141
['get_record', { limit: '10' }, 'limit'],
142+
// [#25302] The executor hands `limit` to the driver as the LIMIT as written.
143+
['get_record', { limit: 2.5 }, 'limit'],
144+
['get_record', { limit: 0 }, 'limit'],
145+
['get_record', { limit: -1 }, 'limit'],
142146
['get_record', { fields: 'name' }, 'fields'],
143147
['create_record', { objectName: 5 }, 'objectName'],
144148
['update_record', { multi: 'true' }, 'multi'],
@@ -336,6 +340,64 @@ describe('[#23260] a value carrying a `{token}` is judged for its type, like any
336340
});
337341
});
338342

343+
/**
344+
* [#25302] `get_record`'s `limit` is a positive integer. The executor does not
345+
* read through `findData`, so the Data API's page-window normalizer never
346+
* judges it: `limit > 1` is handed to `data.find` as the LIMIT as written
347+
* (`2.5` included), and anything else (`0`, `-1`, `0.5`) silently became a
348+
* single-record `findOne`. The contract now refuses the value at every door.
349+
*/
350+
describe('[#25302] get_record limit is a positive integer', () => {
351+
it('a fraction is refused at nodes.1.config.limit, in the contract\'s words', () => {
352+
const config = { ...VALID.get_record, limit: 2.5 };
353+
const issues = issuesOf(flowWith('get_record', config));
354+
expect(issues.map(({ code, path }) => ({ code, path }))).toEqual([{ code: 'custom', path: 'nodes.1.config.limit' }]);
355+
expect(contractSentence('get_record', config, 'limit')).not.toBe('');
356+
expect(issues[0]!.message).toContain(contractSentence('get_record', config, 'limit'));
357+
});
358+
359+
it.each([[2.5], [0], [-1], [0.5], [Number.MAX_SAFE_INTEGER + 2]])(
360+
'defineStack refuses limit %s in its ADR-0112 envelope, at flows.0.nodes.1.config.limit',
361+
(limit) => {
362+
let refusal: { code?: unknown; status?: unknown; issues?: Array<{ path: unknown[]; code: string }> } | undefined;
363+
try {
364+
defineStack({
365+
manifest: { id: 'com.example.limit', name: 'limit', version: '1.0.0', type: 'app', namespace: 'lim' },
366+
objects: [{ name: 'lim_account', label: 'Account', fields: { name: { type: 'text', label: 'Name' } } }],
367+
flows: [flowWith('get_record', { objectName: 'lim_account', limit }, 'lim_probe')],
368+
} as never);
369+
} catch (e) {
370+
refusal = e as typeof refusal;
371+
}
372+
expect(refusal, 'defineStack must refuse the flow').toBeDefined();
373+
expect({ code: refusal!.code, status: refusal!.status }).toEqual({ code: 'STACK_SCHEMA_INVALID', status: 422 });
374+
expect(refusal!.issues!.map((i) => ({ path: i.path.join('.'), code: i.code }))).toEqual([
375+
{ path: 'flows.0.nodes.1.config.limit', code: 'custom' },
376+
]);
377+
},
378+
);
379+
380+
it('the registered `flow` type schema — what the metadata save door validates against — refuses it', () => {
381+
const schema = getMetadataTypeSchema('flow') as unknown as typeof FlowSchema;
382+
const r = schema.safeParse(flowWith('get_record', { ...VALID.get_record, limit: 0 }));
383+
expect(r.success).toBe(false);
384+
expect(r.success ? [] : r.error.issues.map((i) => i.path.join('.'))).toEqual(['nodes.1.config.limit']);
385+
});
386+
387+
it.each([[1], [2], [200]])('CONTROL: limit %s parses, and the judge says nothing', (limit) => {
388+
const config = { ...VALID.get_record, limit };
389+
expect(flowNodeConfigRefusals('get_record', config)).toEqual([]);
390+
const r = FlowSchema.safeParse(flowWith('get_record', config));
391+
expect(r.success, JSON.stringify(r.error?.issues ?? [])).toBe(true);
392+
});
393+
394+
it('CONTROL: a node with no limit parses unchanged', () => {
395+
const { limit: _omitted, ...config } = VALID.get_record;
396+
expect(flowNodeConfigRefusals('get_record', config)).toEqual([]);
397+
expect(FlowSchema.safeParse(flowWith('get_record', config)).success).toBe(true);
398+
});
399+
});
400+
339401
describe('what stays accepted (lit controls)', () => {
340402
it('CONTROL: a valid node of every builtin type parses, and the judge says nothing', () => {
341403
expect(Object.keys(VALID).sort()).toEqual([...getBuiltinNodeConfigContracts().keys()].sort());

‎packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import type { SemanticMigration } from '../../types.js';
1010
// narrows a flow's accept set; no key is removed, so there is no tombstone and
1111
// no RETIRED_KEYS_BY_MAJOR row. There is no D2 conversion either: the platform
1212
// cannot know the value the author meant, and the runtime never ran such a node.
13+
// (#25302) The one exception is `get_record`'s `limit` range: a fraction or a
14+
// count below 1 did run, as the driver's LIMIT as written (`2.5`) or as a
15+
// single-record `findOne` (`0`, `-1`), so its refusal changes what such a node
16+
// did rather than how it fails, and still no conversion can know the count.
1317
//
1418
// No backticks and no pipes in `surface` — build-upgrade-guide.ts renders it
1519
// inside a code span and a table cell.
@@ -21,7 +25,7 @@ export const entry: SemanticMigration = {
2125
+ 'contract refuses — a value of the wrong type (create_record outputVariable 42, a screen field min '
2226
+ 'written as the string 1, get_record limit as a string, update_record multi as a string), a value '
2327
+ 'outside the declared set or range (notify severity loud, screen mode view, loop maxIterations 0, '
24-
+ 'try_catch retry.maxRetries above 10), an empty script function or subflow flowName, or a rule '
28+
+ '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 '
2529
+ 'finding on present keys (a notify template beside an inline title). A value carrying a token in '
2630
+ 'braces is judged like any other (get_record limit written as a token, refused as the wrong type), '
2731
+ '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 = {
3640
+ 'string (`outputVariable: \'taskId\'`), a number where it wants a number (`min: 1`, `limit: 10`, '
3741
+ '`maxIterations: 5`, `timeoutMs: 5000`), a boolean where it wants a boolean (`multi: true`, '
3842
+ '`durable: true`), one of the declared values (`severity: \'warning\'`, `mode: \'edit\'`), or a value '
39-
+ 'inside the declared range. A number or boolean slot takes a LITERAL only: every builtin executor '
43+
+ 'inside the declared range (a `get_record` `limit` is a positive integer: `limit: 1` reads one record, '
44+
+ 'a larger whole number a list of at most that many). A number or boolean slot takes a LITERAL only: every builtin executor '
4045
+ 'parses the config as authored, so a `{token}` template there (`limit: \'{page.size}\'`, '
4146
+ '`maxIterations: \'{cap}\'`) is refused at the build doors as the wrong type, as every run refused '
4247
+ 'it — and at an `http` node\'s `durable` and `timeoutMs`, which `http` once interpolated before it '
@@ -72,7 +77,10 @@ export const entry: SemanticMigration = {
7277
+ 'refused: registered from the metadata registry or `sys_metadata` at boot it is skipped with a '
7378
+ '`warn` naming it, its trigger not armed, while the flows beside it register; a '
7479
+ '`defineStack({ flows })` source throws `StackSchemaInvalidError` for the whole stack; an artifact '
75-
+ 'file is refused whole at load. ADR-0087, ADR-0031.',
80+
+ 'file is refused whole at load. One value in this set used to run rather than fail: a `get_record` '
81+
+ '`limit` that is not a positive integer. The executor reads around the Data API\'s page-window '
82+
+ 'normalizer, so `limit: 2.5` reached the driver as its LIMIT as written, and `0` or `-1` silently read '
83+
+ 'one record through `findOne`. ADR-0087, ADR-0031.',
7684
acceptanceCriteria:
7785
'Run `objectstack validate` over every stack authored in config files, and boot every deployed '
7886
+ 'stack. Each refusal names the node and the key: `FlowSchema.parse` anchors a `custom` issue at '

0 commit comments

Comments
 (0)