diff --git a/.changeset/flow-trigger-key-start-node-guidance.md b/.changeset/flow-trigger-key-start-node-guidance.md new file mode 100644 index 0000000000..5ea39bdc6c --- /dev/null +++ b/.changeset/flow-trigger-key-start-node-guidance.md @@ -0,0 +1,39 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): a top-level flow `trigger` / `triggerType` is now sent to the START node's `config`, not to a `type` rename + +`FlowSchema`'s alias table pointed both keys at `type`, so a flow carrying a +top-level trigger block was refused with the rename `trigger` → `type`. +That rename cannot be taken: `type` is the flow KIND +(`autolaunched` | `record_change` | `schedule` | `screen` | `api`), so an author +who followed the advice landed on +`Invalid option: expected one of "autolaunched"|…` one round later, with the +trigger binding still nowhere — and a `.strict()` refusal carries exactly one +actionable sentence. + +The trigger does not move to `type`. It binds on the START node's `config`, as +`{ objectName, triggerType, condition }` with a `record-*` token such as +`record-after-create` — the shape the automation engine and the authoring-time +`resolveFlowTriggerKind` both read. Both keys are `guidance` entries now, beside +the `object` / `objectName` / `schedule` prescriptions that already name that +config, so the rejection says where the binding really lives instead of +prescribing a name: + +``` +Unrecognized key(s) on this flow: `trigger`. + • `trigger` is not a Flow field — a record-change flow binds its trigger on + the START node's `config` (`{ objectName, triggerType, condition }`, where + `triggerType` is a `record-*` token such as `record-after-create`), not at + the flow top level; the flow-level `type` names the flow kind + (`record_change`), not the binding. +``` + +No accept/reject behaviour changes: a top-level `trigger` / `triggerType` was +refused before and is refused now, and `FlowSchema`'s accepted keys and its +`type` enum are untouched — only the prescription the refusal carries. One +measured consequence of dropping the alias row: the guidance channel matches the +exact authored spelling (case folding is the rename channel's job), so a +non-canonical spelling such as `triggertype` now gets the bare rejection rather +than the rename it cannot take. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index a125ec9ebe..65ac8ff5e3 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -193,7 +193,7 @@ assuming `isSystem` covers it is a documented source of bugs. | Assumption | Reality | Anchor | |:---|:---|:---| -| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:685` | +| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:702` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | | "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9922`–`9939` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 8d05aaa9a0..485bd0a60f 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1547,21 +1547,95 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(issue!.message).toContain('`notAKey`'); }); - it('points builder vocabulary (steps/connections/trigger) at the canonical keys', () => { + it('points builder vocabulary (steps/connections) at the canonical keys', () => { expect(unknownKeyIssue(FlowSchema, { ...minimalFlow, steps: [] })!.message) .toContain('`steps` → `nodes`'); expect(unknownKeyIssue(FlowSchema, { ...minimalFlow, connections: [] })!.message) .toContain('`connections` → `edges`'); - expect(unknownKeyIssue(FlowSchema, { ...minimalFlow, trigger: 'record_change' })!.message) - .toContain('`trigger` → `type`'); }); - it('points a top-level object binding at the START node config', () => { - for (const key of ['object', 'objectName']) { - const message = unknownKeyIssue(FlowSchema, { ...minimalFlow, [key]: 'task' })!.message; + // `trigger` and `triggerType` were ALIASES pointing at `type` — a rename no + // author can take: `type` is the flow KIND enum, so following it lands on + // `Invalid option: expected one of "autolaunched"|…` one round later with the + // binding still nowhere. They are `guidance` entries now, so the rejection + // says where the binding really lives instead of prescribing a name. Both + // directions are pinned: the prescription is present, AND the rename is gone. + it('sends a top-level `trigger` to the START node config, never to a `type` rename', () => { + const message = unknownKeyIssue(FlowSchema, { + ...minimalFlow, + trigger: { type: 'record_change', object: 'task', events: ['create'] }, + })!.message; + expect(message).toContain('START node'); + expect(message).toContain('`{ objectName, triggerType, condition }`'); + expect(message, 'the prescription names a real `record-*` token').toContain('record-after-create'); + expect(message, 'the rename an author cannot take is gone').not.toContain('`trigger` → `type`'); + }); + + it('sends a top-level `triggerType` to the START node config, never to a `type` rename', () => { + const message = unknownKeyIssue(FlowSchema, { + ...minimalFlow, + triggerType: 'record-after-create', + })!.message; + expect(message).toContain('START node'); + expect(message).toContain('`{ objectName, triggerType, condition }`'); + expect(message, 'the rename an author cannot take is gone').not.toContain('`triggerType` → `type`'); + }); + + // The alias table is probed case- and separator-insensitively, so removing + // the `triggertype` row takes every spelling of it with the canonical one. + // `guidance` is exact-spelling by design (case folding is the rename + // channel's job — `shared/suggestions.zod.ts`), so a non-canonical spelling + // now gets the bare rejection: no prescription, and — the point — no + // confidently wrong one either. + it('no spelling of the removed alias renames to `type` any more', () => { + const message = unknownKeyIssue(FlowSchema, { + ...minimalFlow, + triggertype: 'record-after-create', + })!.message; + expect(message).toContain('`triggertype`'); + expect(message).not.toContain('→ `type`'); + }); + + // Why both renames were dead ends, pinned so the guidance above cannot + // quietly turn into correct advice: `type` names the flow KIND and accepts + // no lifecycle-event token at all. + it('`type` accepts no `record-*` event token — the reason neither key renames to it', () => { + const result = FlowSchema.safeParse({ ...minimalFlow, type: 'record-after-create' }); + expect(result.success).toBe(false); + const issue = result.error!.issues.find((i: { code: string }) => i.code === 'invalid_value'); + expect(issue!.message).toContain('expected one of'); + expect(issue!.message).not.toContain('record-'); + }); + + it('points a top-level object/schedule binding at the START node config', () => { + const cases: Array<[string, unknown]> = [ + ['object', 'task'], + ['objectName', 'task'], + ['schedule', '0 8 * * *'], + ]; + for (const [key, value] of cases) { + const message = unknownKeyIssue(FlowSchema, { ...minimalFlow, [key]: value })!.message; expect(message, `\`${key}\` should point at the start node`).toContain('START node'); } }); + + // Positive control for the shape every one of those prescriptions points at: + // the trigger really does bind on the START node's `config`, and a flow that + // writes it there parses. + it('accepts the trigger bound on the START node config — the shape the guidance prescribes', () => { + const result = FlowSchema.safeParse({ + ...minimalFlow, + type: 'record_change', + nodes: [ + { + id: 'start', type: 'start', label: 'Start', + config: { objectName: 'task', triggerType: 'record-after-create' }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + }); + expect(result.success).toBe(true); + }); }); describe('FlowNodeSchema', () => { diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 16604d1a1a..abc5dc708f 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -591,11 +591,28 @@ export const FlowSchema = lazySchema(() => strictObject( connections: 'edges', transitions: 'edges', links: 'edges', - trigger: 'type', - triggertype: 'type', title: 'label', }, guidance: { + // `trigger` / `triggerType` were ALIASES pointing at `type` until an + // author took the advice: `type` is the flow KIND + // (`z.enum(['autolaunched', 'record_change', …])`), so the rename lands + // on `Invalid option: expected one of "autolaunched"|…` one round later, + // with the binding still nowhere — the `inputSchema.optional` case this + // file already names, where a rename would be actively wrong. The trigger + // does not move to `type`; it moves to the START node's `config`, which + // is where `resolveFlowTriggerKind` (`automation/flow-trigger-kind.ts`) + // and the engine's `AutomationEngine.resolveTriggerBinding` read it from. + trigger: + '`trigger` is not a Flow field — a record-change flow binds its trigger on the ' + + 'START node\'s `config` (`{ objectName, triggerType, condition }`, where `triggerType` ' + + 'is a `record-*` token such as `record-after-create`), not at the flow top level; the ' + + 'flow-level `type` names the flow kind (`record_change`), not the binding.', + triggerType: + '`triggerType` is not a Flow field — it belongs on the START node\'s `config` ' + + '(`{ objectName, triggerType, condition }`), where a `record-*` token such as ' + + '`record-after-create` binds the lifecycle event; the flow-level `type` names the ' + + 'flow kind (`record_change`), not an event token.', object: '`object` is not a Flow field — a record-change flow binds its object on the ' + 'START node\'s `config` (`{ objectName, triggerType, condition }`), not at the ' +