diff --git a/.changeset/19900-schedule-caller-param-keys.md b/.changeset/19900-schedule-caller-param-keys.md new file mode 100644 index 00000000000..5361d7a9227 --- /dev/null +++ b/.changeset/19900-schedule-caller-param-keys.md @@ -0,0 +1,14 @@ +--- +'@objectstack/trigger-schedule': patch +'@objectstack/spec': patch +--- + +A scheduled run no longer skips a screen whose field is named `schedule`, `jobId` or `flowName` (#19900). + +The schedule trigger starts each run with three seeds of its own in `params` — `jobId`, `flowName` and `schedule` — and there is no caller behind the run. It stated nothing about that, so a `screen` node's headless verdict inferred who supplied each field from `params`, read those three seeds as the caller's answers, and continued past a screen whose field shared one of the names: the run completed with the trigger's value (for `schedule`, the cron descriptor) as the answer. + +The trigger now sets `AutomationContext.callerParamKeys: []` — "the caller supplied nothing" — which the verdict reads instead of inferring. A screen in a scheduled flow pauses, whatever its fields are named. The three seeds stay in `params`; flows that read them are unaffected. + +`@objectstack/spec`: the `callerParamKeys` TSDoc now names the schedule trigger as a producer that states the empty list, and no longer lists it among the producers that leave the key absent. No type changes. + +This supersedes one sentence of the `callerParamKeys` entry (#19846): the schedule trigger no longer leaves the key absent. Record-change, time-relative and webhook triggers still do. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index fcf3decce85..244a5e46ec4 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -957,9 +957,13 @@ a column named like one of the fields and including when a field is named like one of the seeded id keys. ⛔ `waitForInput: false` remains the wrong tool for the headless case: it skips the form for interactive users too. -A run started any other way — a record-change, schedule, time-relative or -webhook trigger, a `subflow` or `map` child, or code calling the engine directly -— carries no such list, and the screen falls back to inferring the caller's keys +A scheduled run has no caller, and the schedule trigger states exactly that — +an empty list — so its own `jobId`, `flowName` and `schedule` params never +answer a screen field: a screen in a scheduled flow pauses. + +A run started any other way — a record-change, time-relative or webhook +trigger, a `subflow` or `map` child, or code calling the engine directly — +carries no such list, and the screen falls back to inferring the caller's keys from the bag. That inference does not treat a key as the caller's when it is named `recordId` or `Id`, when its value equals what the bag carries under `recordId` or `Id` or the record's `id`, or when the subject diff --git a/packages/services/service-automation/src/screen-input-contract.ts b/packages/services/service-automation/src/screen-input-contract.ts index be5c361d497..ba76f1f7367 100644 --- a/packages/services/service-automation/src/screen-input-contract.ts +++ b/packages/services/service-automation/src/screen-input-contract.ts @@ -280,10 +280,11 @@ export interface HeadlessScreenContext { * * ## Absent: the inference, kept for producers that state nothing * - * A record-change, schedule, time-relative or webhook trigger, a `subflow` / - * `map` child run, or code calling the engine directly carries no signal, and - * for those the verdict below still infers — unchanged, and failing toward a - * pause. + * A record-change, time-relative or webhook trigger, a `subflow` / `map` child + * run, or code calling the engine directly carries no signal, and for those the + * verdict below still infers — unchanged, and failing toward a pause. (The + * schedule trigger has no caller and states `[]`, #19900, so its `jobId` / + * `flowName` / `schedule` seeds never reach the inference.) * * This distinction is the whole safety story of {@link judgeHeadlessScreen}, * because the params bag a flow action reaches the engine with is NOT the diff --git a/packages/spec/src/contracts/automation-context-caller-param-keys.pin.test.ts b/packages/spec/src/contracts/automation-context-caller-param-keys.pin.test.ts index 88567935f29..1cad94512f7 100644 --- a/packages/spec/src/contracts/automation-context-caller-param-keys.pin.test.ts +++ b/packages/spec/src/contracts/automation-context-caller-param-keys.pin.test.ts @@ -13,7 +13,8 @@ * not a bag of values, and not a boolean per key. * 2. **Additive.** A context literal without it still type-checks, so no * existing `IAutomationService.execute` caller moves, and an absent key is - * what every producer other than the two doors sends. + * what every producer other than the two doors and the schedule trigger + * (which states `[]`, #19900) sends. * 3. **An empty list is a legal value** — "the caller supplied nothing" — and * it is a different value from an absent key. * diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index 9d375e34343..4e5fd3fd473 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/packages/spec/src/contracts/automation-service.ts @@ -173,8 +173,13 @@ export interface AutomationContext { * a client that mirrors the row id into `params.recordId` is addressing the * row, not answering a question the flow asks. * + * One producer with no caller at all states the empty list: the schedule + * trigger (`ScheduleTrigger` in `@objectstack/trigger-schedule`, #19900). + * Its `jobId`, `flowName` and `schedule` keys in {@link params} are its own + * seeds, so none of them answers a question the flow asks. + * * Absent means the producer does not state provenance: a record-change, - * schedule, time-relative or webhook trigger, a `subflow` or `map` child run + * time-relative or webhook trigger, a `subflow` or `map` child run * (those two nodes drop the parent's list from the child context, because it * describes the PARENT's bag), or code calling `execute` directly. Its * consumer, the `screen` node's headless verdict in diff --git a/packages/triggers/trigger-schedule/src/schedule-caller-param-keys-e2e.test.ts b/packages/triggers/trigger-schedule/src/schedule-caller-param-keys-e2e.test.ts new file mode 100644 index 00000000000..24d12a4ffb3 --- /dev/null +++ b/packages/triggers/trigger-schedule/src/schedule-caller-param-keys-e2e.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19900] A scheduled run's platform seeds never answer a screen field. + * + * The schedule trigger starts a run with three seeds in `params` — `jobId`, + * `flowName` and `schedule` — and there is no caller behind it at all. A + * `screen` node on that headless run asks whether the caller already answered + * each of its fields (`judgeHeadlessScreen` in `@objectstack/service-automation`). + * With no statement from the producer, that answer is INFERRED from `params`, + * and the inference can only rule out the row-id and record seeds the other + * doors write: the schedule trigger's three names read as the caller's own + * answers, so a screen with a required field named `schedule` was skipped and + * the run completed with the cron descriptor as the "answer". + * + * The producer now states it: `callerParamKeys: []` — "the caller supplied + * nothing", which the verdict reads instead of inferring (#19846). + * + * The rig is the one `schedule-runas-e2e.test.ts` uses: the REAL + * `ScheduleTrigger` bound through the REAL `AutomationEngine`, the job fired by + * hand in place of the cron tick. The result read is the one the engine + * returned to the trigger's own callback — not a hand-made context. + * + * ⚠️ `@objectstack/service-automation` resolves to its `dist/` here (no alias: + * `KNOWN_UNALIASED_TEST_IMPORTS` in `scripts/check-test-source-alias.mjs`), so + * the engine half is the built one. The half this file pins — the context the + * trigger builds — is `./schedule-trigger.js`, read from source. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { AutomationEngine, installBuiltinNodes } from '@objectstack/service-automation'; +import type { AutomationContext, JobSchedule, JobHandler } from '@objectstack/spec/contracts'; +import { ScheduleTrigger } from './schedule-trigger.js'; +import { withScheduledWorkOn } from './deployment-switch.test-support.js'; + +function silentLogger(): any { + const l: any = { info() {}, warn() {}, error() {}, debug() {} }; + l.child = () => l; + return l; +} +const flush = () => new Promise((r) => setTimeout(r, 0)); + +/** Fake IJobService slice with a deterministic `fire()` standing in for the cron tick. */ +function fakeJobService() { + const jobs = new Map(); + return { + service: { + async schedule(name: string, schedule: JobSchedule, handler: JobHandler) { + jobs.set(name, { schedule, handler }); + }, + async cancel(name: string) { + jobs.delete(name); + }, + }, + has: (name: string) => jobs.has(name), + fire: async (name: string, jobId = 'tick-1') => { + await jobs.get(name)?.handler({ jobId } as never); + }, + }; +} + +type Field = { name: string; label: string; type: string; required?: boolean }; + +const CRON: JobSchedule = { type: 'cron', expression: '0 2 * * *' } as JobSchedule; + +/** + * `start → screen_1 → end` on a `type: 'schedule'` flow, each screen field an + * `isInput` variable — so the seed of the same name IS bound into it, which is + * the only way the skip could happen. + */ +function scheduledScreenFlow(name: string, fields: Field[]) { + return { + name, + label: name, + type: 'schedule', + runAs: 'system', + status: 'active', + version: 1, + variables: fields.map((f) => ({ name: f.name, type: 'text', isInput: true, isOutput: true })), + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { schedule: CRON } }, + { id: 'screen_1', type: 'screen', label: 'Ask', config: { fields } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'screen_1', type: 'default' }, + { id: 'e2', source: 'screen_1', target: 'end', type: 'default' }, + ], + } as never; +} + +/** + * Bind the flow through the real trigger, fire one tick, and hand back what + * the trigger passed to the engine and what the engine answered it with. + */ +async function fireOnce(flowName: string, fields: Field[]) { + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, { logger: silentLogger(), getService: () => undefined } as never); + const execute = vi.spyOn(engine, 'execute'); + engine.registerFlow(flowName, scheduledScreenFlow(flowName, fields)); + const job = fakeJobService(); + engine.registerTrigger(new ScheduleTrigger(() => job.service, silentLogger())); + await flush(); // let ScheduleTrigger.start() schedule the job + + const jobName = `flow-schedule:${flowName}`; + expect(job.has(jobName), 'the flow was not bound to the schedule trigger').toBe(true); + await job.fire(jobName, 'tick-42'); + + expect(execute).toHaveBeenCalledTimes(1); + const context = execute.mock.calls[0][1] as AutomationContext; + const result = await execute.mock.results[0].value; + return { engine, context, result }; +} + +// [#17396] The switch is OFF by default in every posture; nothing binds without it. +withScheduledWorkOn('single'); + +describe('[#19900] a scheduled run\'s platform seeds never answer a screen field', () => { + it('the trigger states that its run has no caller: `callerParamKeys` is an empty list', async () => { + const { context } = await fireOnce('nightly_ask', [ + { name: 'schedule', label: 'Schedule', type: 'text', required: true }, + ]); + // The seeds are still there — flows read them — they are just not a caller's. + expect(context.params).toMatchObject({ jobId: 'tick-42', flowName: 'nightly_ask', schedule: CRON }); + expect(context.callerParamKeys).toEqual([]); + }); + + it.each(['schedule', 'jobId', 'flowName'])( + 'a required screen field named `%s` pauses the run instead of taking the seed as its answer', + async (seed) => { + const { engine, result } = await fireOnce('nightly_ask', [ + { name: seed, label: seed, type: 'text', required: true }, + ]); + // One object, so a skip prints what the run completed WITH. + expect(result).toMatchObject({ status: 'paused', screen: { nodeId: 'screen_1' } }); + expect(engine.listSuspendedRuns()).toEqual([ + expect.objectContaining({ flowName: 'nightly_ask', nodeId: 'screen_1' }), + ]); + }, + ); + + it('an all-optional screen named after every seed pauses too — the seeds do not "drive" it', async () => { + const { result } = await fireOnce('nightly_ask', [ + { name: 'schedule', label: 'Schedule', type: 'text' }, + { name: 'jobId', label: 'Job', type: 'text' }, + { name: 'flowName', label: 'Flow', type: 'text' }, + ]); + expect(result).toMatchObject({ status: 'paused', screen: { nodeId: 'screen_1' } }); + }); + + it('control: the same flow on the same engine continues when a context names `schedule` as a caller key', async () => { + // The pause above is the signal's doing, not "a screen in a schedule flow + // always pauses": a door that states real caller keys still answers it. + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, { logger: silentLogger(), getService: () => undefined } as never); + engine.registerFlow('nightly_ask', scheduledScreenFlow('nightly_ask', [ + { name: 'schedule', label: 'Schedule', type: 'text', required: true }, + ])); + const result = await engine.execute('nightly_ask', { + event: 'schedule', + params: { jobId: 'tick-42', flowName: 'nightly_ask', schedule: '0 2 * * *' }, + callerParamKeys: ['schedule'], + }); + expect(result.status).not.toBe('paused'); + expect(result.success).toBe(true); + expect(result.output).toMatchObject({ schedule: '0 2 * * *' }); + expect(engine.listSuspendedRuns()).toEqual([]); + }); +}); diff --git a/packages/triggers/trigger-schedule/src/schedule-trigger.ts b/packages/triggers/trigger-schedule/src/schedule-trigger.ts index fecaf492503..5d67f8394de 100644 --- a/packages/triggers/trigger-schedule/src/schedule-trigger.ts +++ b/packages/triggers/trigger-schedule/src/schedule-trigger.ts @@ -808,6 +808,15 @@ export class ScheduleTrigger implements FlowTrigger { flowName: binding.flowName, schedule, }, + // [#19900] A tick has no caller, so no key above is a + // caller's answer — all three are this trigger's own seeds. + // Stated rather than left to inference: a `screen` node's + // headless verdict reads this list when present (#19846), + // and without it inferred `jobId` / `flowName` / `schedule` + // as supplied, skipping a screen whose field shared a name. + // ⛔ An empty list, never an absent key — absent means + // "not stated", and the inference answers that. + callerParamKeys: [], }; await callback(ctx); if (key) await this.settleDispatch(binding.flowName, key, 'succeeded');