From f91c686410f993fc06fde0ef23463350f4827618 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:07:33 +0000 Subject: [PATCH 1/9] wip(#15705): headless screen satisfaction + flow input params Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- packages/runtime/src/action-execution.ts | 86 ++++++++++++- packages/runtime/src/domains/mcp.ts | 23 +++- .../src/builtin/screen-nodes.ts | 38 +++++- .../src/screen-input-contract.ts | 121 ++++++++++++++++++ 4 files changed, 261 insertions(+), 7 deletions(-) diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 9fcb7bd00a..b947296b0c 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -981,7 +981,7 @@ export function actionLooksDestructive(_deps: ActionExecutionDeps, action: any): return Boolean(action?.mode === 'delete' || action?.variant === 'danger'); } -export function summarizeAction(deps: ActionExecutionDeps, action: any, obj: any, objectName: string): any { +export function summarizeAction(deps: ActionExecutionDeps, action: any, obj: any, objectName: string, flow?: any): any { // [#15079] `operation` before `type`, on the LISTING face. A declarative // update always requires a current record — that is contract point 7, and // the executor refuses without one — so the answer cannot be left to @@ -999,7 +999,7 @@ export function summarizeAction(deps: ActionExecutionDeps, action: any, obj: any const description = (typeof action?.ai?.description === 'string' ? action.ai.description : undefined) ?? (typeof action?.label === 'string' ? action.label : undefined); - const params = summarizeActionParams(deps, action, obj); + const params = summarizeActionParams(deps, action, obj, flow); return { name: action.name, objectName, @@ -1029,7 +1029,7 @@ export function jsonTypeOf(_deps: ActionExecutionDeps, t: string | undefined): ' } } -export function summarizeActionParams(deps: ActionExecutionDeps, action: any, obj: any): any[] { +export function summarizeActionParams(deps: ActionExecutionDeps, action: any, obj: any, flow?: any): any[] { const fields: Record = obj?.fields ?? {}; const out: any[] = []; for (const p of (Array.isArray(action?.params) ? action.params : [])) { @@ -1055,9 +1055,89 @@ export function summarizeActionParams(deps: ActionExecutionDeps, action: any, ob ...(enumVals.length > 0 ? { enum: enumVals } : {}), }); } + // [#15705] A FLOW action's input contract is its flow's `isInput` + // variables, not `action.params` — a flow-typed action almost never + // declares `params`, so this listing answered with no `params` key at all + // while the MCP `list_actions` tool description promised "its input + // parameters". An agent could see the action, could invoke it, and had no + // way to learn a single input name. + // + // Second, never first: a declaration the AUTHOR wrote on the action wins + // outright, so this can only fill a silence. `flow` is optional and the + // caller resolves it (`domains/mcp.ts` asks the automation service's + // `getFlow`), which keeps this function pure and leaves every existing + // 3-argument call site — and every non-flow action — byte-identical. + if (out.length === 0) out.push(...summarizeFlowInputParams(deps, flow)); + return out; +} + +/** + * A screen flow's input contract, projected onto the same param shape + * {@link summarizeActionParams} emits for a declared param (#15705). + * + * The flow's `isInput` variables ARE the contract — they are what + * `seedDeclaredVariables` binds from the caller's `params`, so their names are + * exactly the keys an invoker must send. The variable declaration carries only + * `name` / `type` / `defaultValue`, so everything an agent needs beyond the + * name (`label`, `required`, select `options`) is read off the screen node + * that collects the variable — the same field spec a paused run surfaces. + * + * `required` comes from the screen field alone: a flow variable has no + * `required` key, and inferring one from "declares no `defaultValue`" would + * invent a contract the author never wrote. A variable no screen collects is + * still listed — it is a real input, and omitting it would hide the very names + * this exists to publish — just without the screen-only enrichments. + */ +export function summarizeFlowInputParams(deps: ActionExecutionDeps, flow: any): any[] { + const variables: any[] = Array.isArray(flow?.variables) ? flow.variables : []; + if (variables.length === 0) return []; + const screenFields = collectScreenFieldSpecs(flow); + const out: any[] = []; + for (const v of variables) { + const name: unknown = v?.name; + if (v?.isInput !== true || typeof name !== 'string' || !name) continue; + const field = screenFields.get(name); + const type = jsonTypeOf(deps, field?.type ?? v?.type); + const description = typeof field?.label === 'string' && field.label ? field.label : undefined; + const enumVals = Array.isArray(field?.options) + ? field.options + .map((o: any) => (typeof o === 'string' ? o : o?.value)) + .filter((x: any): x is string => typeof x === 'string') + : []; + out.push({ + name, + type, + required: field?.required === true, + ...(description ? { description } : {}), + ...(enumVals.length > 0 ? { enum: enumVals } : {}), + }); + } return out; } +/** + * Every screen field a flow declares, by field name, first declaration + * winning. Walks ALL `screen` nodes rather than just the first: a multi-step + * wizard collects its inputs across several screens, and a contract that + * stopped at screen one would publish a subset while looking complete. + * + * Object-form screens contribute nothing by construction — their `fields` is + * empty because the client renders the object's own form — so they are simply + * skipped rather than special-cased. + */ +function collectScreenFieldSpecs(flow: any): Map { + const byName = new Map(); + for (const node of Array.isArray(flow?.nodes) ? flow.nodes : []) { + if (node?.type !== 'screen') continue; + for (const field of Array.isArray(node?.config?.fields) ? node.config.fields : []) { + const name: unknown = field?.name; + if (typeof name !== 'string' || !name || byName.has(name)) continue; + byName.set(name, field); + } + } + return byName; +} + /** * Resolve an action's declared `params[]` to their effective value-shape * inputs (ADR-0104 D2). A field-backed param inherits type/multiple/ diff --git a/packages/runtime/src/domains/mcp.ts b/packages/runtime/src/domains/mcp.ts index b6e1a461cf..3503c99d5d 100644 --- a/packages/runtime/src/domains/mcp.ts +++ b/packages/runtime/src/domains/mcp.ts @@ -627,7 +627,14 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon // identity forwarded. No `@objectstack/service-ai`. listActions: async () => { const meta: any = await getMeta(); - const hasAutomation = Boolean(await actionExec.resolveAutomationService(deps, context, envId)); + // [#15705] The service instance, not just its presence: a + // flow-typed action's input contract lives in the FLOW, and + // `getFlow` is the declared door onto it (`IAutomationService`, + // the same probe `dispatchFlowAction` uses before it dispatches). + // Without it `list_actions` answered with no `params` for every + // flow action while promising "its input parameters". + const automation: any = await actionExec.resolveAutomationService(deps, context, envId); + const hasAutomation = Boolean(automation); const out: any[] = []; for (const { action, objectName, obj } of await actionExec.collectActionDeclarations(deps, meta)) { if (!objectName || isSystemObjectName(objectName)) continue; // fail-closed on sys_* @@ -639,7 +646,19 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon if (actionExec.actionAiExposureError(deps, action)) continue; // Hide actions the caller is not permitted to run. if (actionExec.actionPermissionError(deps, action, ec)) continue; - out.push(actionExec.summarizeAction(deps, action, obj, objectName)); + // Resolved per flow action, and only for one: a service that + // omits the optional `getFlow` — or a flow the registry does + // not hold — simply summarizes as before, since the fallback + // is "no `params` key", exactly today's answer. + let flow: any; + if (action?.type === 'flow' && typeof action?.target === 'string' && typeof automation?.getFlow === 'function') { + try { + flow = await automation.getFlow(action.target); + } catch { + flow = undefined; // a registry that cannot answer is not a listing failure + } + } + out.push(actionExec.summarizeAction(deps, action, obj, objectName, flow ?? undefined)); } return out; }, diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts index 180b6453d9..db5be8148e 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.ts @@ -6,6 +6,7 @@ import type { ScreenConfigParsed, ScriptConfigParsed } from '@objectstack/spec/a import type { AutomationEngine } from '../engine.js'; import { interpolate } from './template.js'; import { parseNodeConfig } from './parse-config.js'; +import { judgeHeadlessScreen } from '../screen-input-contract.js'; /** * Screen / Script built-in nodes — 'screen' and 'script' executors. @@ -18,7 +19,13 @@ import { parseNodeConfig } from './parse-config.js'; * to render; the run continues via `resume()` with the collected values (set * as bare flow variables). A field-less screen — or one with * `waitForInput === false` — stays a server pass-through (input vars, if any, - * are already injected from `context.params`). + * are already injected from `context.params`). Since #15705 a screen the + * RUN'S CALLER already answered is a fourth pass-through: when the caller + * supplied this screen's own fields and every `required` one is bound, there + * is nothing left to collect, so the run continues rather than parking on a + * form a headless invoker cannot submit. `judgeHeadlessScreen` owns that + * verdict and refuses on every uncertainty, so an interactive run is + * untouched. * - 'script' nodes call a registered function (#1870): `config.function` names * it, `engine.resolveFunction()` resolves it, and the host bridges that to * `bundle.functions` / `defineStack({ functions })`. A name that resolves to @@ -162,7 +169,34 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext const hasFields = rawFields.length > 0; // Suspend to collect input when the screen declares fields, or opts in // explicitly. `waitForInput === false` forces a server pass-through. - const shouldPause = cfg.waitForInput === true || (hasFields && cfg.waitForInput !== false); + const wantsPause = cfg.waitForInput === true || (hasFields && cfg.waitForInput !== false); + // #15705 — a screen whose inputs the CALLER already supplied is + // answered, so the run continues instead of parking on a form nobody + // will ever submit. A headless invoker (`run_action` over MCP) seeds + // the flow's `isInput` variables and then has no resume verb; before + // this, every screen-typed flow action was a dead end for it. + // + // Three deliberate narrowings, each keeping an existing behaviour whole: + // + // - `waitForInput === true` is NOT overridden. That flag is the + // author's explicit "show this", and its documented job is the + // field-less message / confirmation screen — a screen that collects + // nothing and would therefore be VACUOUSLY satisfiable. Honouring it + // keeps a confirmation step from being skipped by a bag it never + // asked for. + // - `hasFields` is required, for the same vacuity reason stated from + // the other side: no declared fields means no contract to satisfy. + // - the verdict itself refuses on every uncertainty + // (`judgeHeadlessScreen`), so an interactive run — which supplies + // none of the screen's own fields — takes the untouched path. + // + // ⛔ This does NOT make every screen flow completable over MCP: a call + // that omits the inputs still parks, and nothing on that surface can + // continue it. That half is a resume verb, and it is not this change. + const headless = wantsPause && hasFields && cfg.waitForInput !== true + ? judgeHeadlessScreen(rawFields, variables, context) + : undefined; + const shouldPause = wantsPause && headless?.satisfied !== true; if (!shouldPause) { return { success: true }; } diff --git a/packages/services/service-automation/src/screen-input-contract.ts b/packages/services/service-automation/src/screen-input-contract.ts index a329b0f604..a988f828aa 100644 --- a/packages/services/service-automation/src/screen-input-contract.ts +++ b/packages/services/service-automation/src/screen-input-contract.ts @@ -142,3 +142,124 @@ export function validateScreenInputs( export function declaredScreenFieldNames(fields: readonly ScreenFieldSpec[]): string[] { return fields.map((f) => f?.name).filter((n): n is string => typeof n === 'string' && n.length > 0); } + +/** + * A screen field, reduced to the three keys a satisfaction verdict turns on. + * Structurally a {@link ScreenFieldSpec} subset, so the node executor can hand + * its parsed `config.fields` straight in. + */ +export interface ScreenFieldContract { + name: string; + required?: boolean; + visibleWhen?: string; +} + +/** Why a screen was (or was not) satisfied without showing it — see {@link judgeHeadlessScreen}. */ +export interface HeadlessScreenVerdict { + /** `true` ⇒ the run may continue past this screen without suspending. */ + satisfied: boolean; + /** Declared field names whose value this run's CALLER supplied (provenance-checked). */ + supplied: string[]; + /** Required fields with no usable bound value — the reason a candidate was refused. */ + missing: string[]; +} + +const NOTHING_SUPPLIED: HeadlessScreenVerdict = { satisfied: false, supplied: [], missing: [] }; + +/** + * Whether a screen field's value in `context.params` came from the run's + * CALLER rather than from the subject record the dispatcher seeded. + * + * This distinction is the whole safety story of {@link judgeHeadlessScreen}, + * because the params bag a flow action reaches the engine with is NOT the + * caller's bag: `seedFlowActionParams` (`@objectstack/runtime`) returns + * `{ ...record, recordId, Id, ...params }`, so every column of the + * subject row is in there whether the caller named it or not. Reading "the key + * is in `params`" as "the caller supplied it" would let an INTERACTIVE console + * run — which supplies nothing — skip a screen whose field happens to share a + * name with a column of the record it was launched from. + * + * Two legs, either of which proves caller provenance: + * + * - the record has no such key at all ⇒ the record leg cannot be the source; + * - the record HAS the key but `params` holds a different value ⇒ the + * caller's bag overwrote it. `{ ...record }` copies the record's own value + * by reference/primitive, so a run that supplied nothing is `Object.is`-equal + * here, always. Equality is therefore "indistinguishable", not "caller-set". + * + * The ambiguous case (same key, same value) resolves to NOT caller-supplied, + * which costs a headless run a pause it might have been allowed to skip and + * costs an interactive run nothing. That asymmetry is deliberate: every + * uncertainty in this module must land on today's behaviour. + */ +function callerSupplied( + name: string, + context: { params?: Record; record?: Record } | undefined, +): boolean { + const params = context?.params; + if (!params || params[name] === undefined) return false; + const record = context?.record; + if (!record || !Object.prototype.hasOwnProperty.call(record, name)) return true; + return !Object.is(params[name], record[name]); +} + +/** + * Can this screen be treated as already answered, and the run continued, + * without suspending to show it? (#15705) + * + * The defect this answers: an `ai.exposed` action whose target is a screen + * flow could be STARTED over MCP and never finished. `run_action` seeds the + * flow's `isInput` variables from the caller's `params` — correctly — and the + * screen node then suspended anyway, because the only inputs to that decision + * were "does the node declare fields" and the author's `waitForInput` flag. + * The MCP tool set has no resume verb, so the run parked forever. + * + * ⛔ NOT a general "skip screens" switch. Three conditions must ALL hold, and + * the verdict is `false` the moment any of them is unproven: + * + * 1. **The caller supplied at least one of THIS screen's declared fields** + * ({@link callerSupplied}). Without this leg a screen whose fields are all + * optional would be vacuously "satisfied" and would stop rendering for + * everyone — the loudest way to break the interactive path. A run that + * named none of this screen's fields is not driving it, so it pauses. + * 2. **Every `required` field has a usable value bound** in the live flow + * variables — judged by {@link validateScreenInputs}, the same function + * the resume door enforces the same contract with, so "present" cannot + * drift into two meanings (an empty string is absent on both). + * 3. Only caller-supplied names enter the bag, so a required field bound + * from the record, from a prior node or from a declared `defaultValue` + * does NOT count as answered. Optional fields are free to come from + * anywhere — they constrain nothing. + * + * **`visibleWhen` is enforced here, the OPPOSITE of the resume door**, and the + * asymmetry is the point rather than an oversight. On resume, an unevaluable + * predicate must not fire `required`: the client is the authority on what the + * user was shown, and demanding a hidden field dead-ends a run at Submit + * (#3528). Here the server has no client and no collected values, so it cannot + * evaluate the predicate either — but refusing costs nothing except a pause, + * which is exactly what this screen does today. So a conditional required field + * the caller did not name keeps the screen interactive. + */ +export function judgeHeadlessScreen( + fields: readonly ScreenFieldContract[], + variables: ReadonlyMap, + context: { params?: Record; record?: Record } | undefined, +): HeadlessScreenVerdict { + const declared = fields.filter((f) => typeof f?.name === 'string' && f.name.length > 0); + if (declared.length === 0) return NOTHING_SUPPLIED; + + const supplied: string[] = []; + const bag: Record = {}; + for (const field of declared) { + if (!callerSupplied(field.name, context)) continue; + supplied.push(field.name); + bag[field.name] = variables.get(field.name); + } + // Condition 1 — nobody drove this screen, so it stays interactive. + if (supplied.length === 0) return NOTHING_SUPPLIED; + + // Condition 2/3 — `unknown_field` cannot fire: every bag key is a declared + // field by construction, so every issue returned here is a missing `required`. + const issues = validateScreenInputs(declared, bag, () => true); + return { satisfied: issues.length === 0, supplied, missing: issues.map((i) => i.field) }; +} From f2a6f7c501d3bf31b53b97e06f745afdb8d2fe27 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:14:03 +0000 Subject: [PATCH 2/9] test(#15705): pin headless screen satisfaction and its interactive controls Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../screen-headless-satisfaction.test.ts | 276 ++++++++++++++++++ 1 file changed, 276 insertions(+) create mode 100644 packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts diff --git a/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts new file mode 100644 index 0000000000..331556df09 --- /dev/null +++ b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts @@ -0,0 +1,276 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A screen the CALLER already answered no longer parks the run (#15705). + * + * The reported dead end: an `ai.exposed` action whose target is a screen flow + * can be STARTED over MCP and never finished. `run_action` seeds the flow's + * `isInput` variables from the caller's `params` — `seedFlowActionParams` does + * that correctly — and the screen node suspended anyway, because the only + * inputs to `shouldPause` were "does the node declare fields" and the author's + * `waitForInput` flag. The MCP tool set has no resume verb, so the run parked + * on the screen with nothing able to continue it: `ai.exposed` meant "the agent + * can invoke this", not "the agent can complete this". + * + * ## What this file pins, and why the controls outnumber the fix + * + * The fix is one line of predicate; the risk is entirely in the OTHER runs that + * reach it. A screen node is also entered by interactive console runs, by + * record-change and scheduled triggers, and by region bodies — and the failure + * mode of getting this wrong is silent: a screen that stops rendering for a + * human. So every narrowing in `judgeHeadlessScreen` has a control here, and + * the interactive-still-pauses control is the load-bearing one. + * + * The sharpest of them is `seeded params`: the bag the engine receives from a + * flow ACTION is not the caller's bag. `seedFlowActionParams` returns + * `{ ...record, recordId, Id, ...params }`, so every column of the + * subject row arrives in `context.params` whether the caller named it or not. + * Reading "the key is in params" as "the caller supplied it" would let an + * interactive console run — which supplies nothing — skip a screen whose field + * shares a name with a column of the record it was launched from. Those runs + * are reproduced here through the real seeding shape, not a hand-made bag. + * + * ⛔ This does NOT claim screen flows are completable over MCP in general: a + * call that omits the inputs still parks (pinned below), and nothing on that + * surface can resume it. That half is a resume verb and is not this change. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { AutomationEngine } from '../engine.js'; +import { installBuiltinNodes } from './index.js'; +import type { AutomationContext } from '@objectstack/spec/contracts'; + +function silentLogger() { + return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any; +} +function ctx() { + return { logger: silentLogger(), getService() { return undefined; } } as any; +} + +/** + * The card's specimen, reduced to the platform facts: a `screen` flow whose + * first node after `start` collects the same names the flow declares as + * `isInput` variables. `subject` and `dueDate` are required, `notes` is not. + * All three are `isOutput` too, so a completed run reports what actually bound + * — "it continued" and "it continued with the caller's values" are different + * claims and the second is the one worth making. + */ +function followupFlow(overrides: Record = {}) { + return { + name: 'schedule_followup', + label: 'Schedule Follow-up', + type: 'screen', + status: 'active', + version: 1, + variables: [ + { name: 'subject', type: 'text', isInput: true, isOutput: true }, + { name: 'dueDate', type: 'text', isInput: true, isOutput: true }, + { name: 'notes', type: 'text', isInput: true, isOutput: true }, + ], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'screen_1', type: 'screen', label: 'Schedule Follow-up', + config: { + fields: [ + { name: 'subject', label: 'Subject', type: 'text', required: true }, + { name: 'dueDate', label: 'Due date', type: 'date', required: true }, + { name: 'notes', label: 'Notes', type: 'text' }, + ], + ...overrides, + }, + }, + { 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' }, + ], + }; +} + +/** + * The params bag a flow ACTION actually reaches the engine with — the subject + * row first, the caller's explicit params last, exactly as + * `seedFlowActionParams` (`@objectstack/runtime`) composes it. Reproduced here + * rather than imported so this package's pins do not depend on the other + * package's build; the shape is asserted against the real producer's + * documented contract in its own suite. + */ +function actionContext( + record: Record, + params: Record, +): AutomationContext { + return { + record, + object: 'crm_lead', + params: { ...record, recordId: record.id, crmLeadId: record.id, ...params }, + } as AutomationContext; +} + +const LEAD = { id: 'lead_1', name: 'Acme', company: 'Acme Inc' }; + +describe('screen headless satisfaction (#15705)', () => { + let engine: AutomationEngine; + + beforeEach(() => { + engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, ctx()); + }); + + function register(overrides: Record = {}, flow = followupFlow(overrides)) { + engine.registerFlow('schedule_followup', flow as any); + } + + // ── The fix ─────────────────────────────────────────────────────────── + + it('continues past the screen when the caller supplied every required field', async () => { + register(); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { + subject: 'Call Acme back', dueDate: '2026-09-09', notes: 'left voicemail', + })); + expect(res.status).not.toBe('paused'); + expect(res.screen).toBeUndefined(); + expect(res.success).toBe(true); + // The values the caller sent are what the run carried — not merely that + // it did not stop. + expect(res.output).toMatchObject({ + subject: 'Call Acme back', dueDate: '2026-09-09', notes: 'left voicemail', + }); + }); + + it('continues when the caller supplied the REQUIRED fields and left an optional one out', async () => { + register(); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { + subject: 'Call Acme back', dueDate: '2026-09-09', + })); + expect(res.status).not.toBe('paused'); + expect(res.success).toBe(true); + expect(res.output).toMatchObject({ subject: 'Call Acme back', notes: undefined }); + }); + + // ── The control that matters: interactive runs are untouched ────────── + + it('CONTROL — an interactive run (no params) still pauses and still renders the form', async () => { + register(); + const res = await engine.execute('schedule_followup', actionContext(LEAD, {})); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + expect(res.screen?.fields.map((f) => f.name)).toEqual(['subject', 'dueDate', 'notes']); + expect(res.screen?.fields.find((f) => f.name === 'subject')?.required).toBe(true); + }); + + it('CONTROL — a run with NO context at all still pauses (trigger / schedule shape)', async () => { + register(); + const res = await engine.execute('schedule_followup', {} as AutomationContext); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + }); + + /** + * The provenance leg, stated as the regression it prevents. Here the SUBJECT + * ROW carries columns named exactly like the screen's required fields, so + * the dispatcher's `{ ...record }` seed puts both names in `context.params` + * for a run that supplied nothing. Treating "in params" as "caller supplied" + * would skip this screen for a human who pressed a button. + */ + it('CONTROL — record columns that collide with screen field names do NOT satisfy the screen', async () => { + register(); + const collidingLead = { ...LEAD, subject: 'row value', dueDate: '2026-01-01' }; + const res = await engine.execute('schedule_followup', actionContext(collidingLead, {})); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + }); + + it('a caller that OVERRIDES a colliding column is still caller-supplied and continues', async () => { + register(); + const collidingLead = { ...LEAD, subject: 'row value', dueDate: '2026-01-01' }; + const res = await engine.execute('schedule_followup', actionContext(collidingLead, { + subject: 'Call Acme back', dueDate: '2026-09-09', + })); + expect(res.status).not.toBe('paused'); + expect(res.output).toMatchObject({ subject: 'Call Acme back', dueDate: '2026-09-09' }); + }); + + it('CONTROL — a partially supplied screen still pauses (one required field missing)', async () => { + register(); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { + subject: 'Call Acme back', + })); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + }); + + it('CONTROL — an empty string does not answer a required field', async () => { + register(); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { + subject: 'Call Acme back', dueDate: ' ', + })); + expect(res.status).toBe('paused'); + }); + + // ── Vacuity guards: a screen with nothing to satisfy must not be skipped ── + + it('CONTROL — an explicit `waitForInput: true` still pauses even when fully supplied', async () => { + register({ waitForInput: true }); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { + subject: 'Call Acme back', dueDate: '2026-09-09', + })); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + }); + + it('CONTROL — a message-only screen (no fields) still pauses; a bag cannot vacuously answer it', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config = { title: 'Confirm', waitForInput: true }; + register({}, flow); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { + subject: 'Call Acme back', dueDate: '2026-09-09', + })); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + }); + + it('CONTROL — an all-optional screen still pauses when the caller named none of its fields', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'notes', label: 'Notes', type: 'text' }]; + register({}, flow); + const res = await engine.execute('schedule_followup', actionContext(LEAD, {})); + expect(res.status).toBe('paused'); + }); + + it('an all-optional screen the caller DID name is answered and continues', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'notes', label: 'Notes', type: 'text' }]; + register({}, flow); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { notes: 'left voicemail' })); + expect(res.status).not.toBe('paused'); + expect(res.output).toMatchObject({ notes: 'left voicemail' }); + }); + + /** + * `visibleWhen` is enforced HERE and deliberately not on the resume door. + * The server has no client and no collected values, so it cannot evaluate + * the predicate — but refusing costs only a pause, which is what this + * screen does today anyway. The resume door makes the opposite call for the + * opposite reason: there, demanding a hidden field dead-ends a run at + * Submit (#3528). + */ + it('CONTROL — a conditional required field the caller did not name keeps the screen interactive', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [ + { name: 'subject', label: 'Subject', type: 'text', required: true }, + { name: 'notes', label: 'Reason', type: 'text', required: true, visibleWhen: "subject == 'escalate'" }, + ]; + register({}, flow); + const res = await engine.execute('schedule_followup', actionContext(LEAD, { subject: 'Call Acme back' })); + expect(res.status).toBe('paused'); + }); + + it('CONTROL — `waitForInput: false` is still a pass-through, unchanged', async () => { + register({ waitForInput: false }); + const res = await engine.execute('schedule_followup', actionContext(LEAD, {})); + expect(res.status).not.toBe('paused'); + expect(res.success).toBe(true); + }); +}); From 24f50bb0bafb9daa97d02ef7872bcb243263e084 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:17:13 +0000 Subject: [PATCH 3/9] test(#15705): pin the flow input contract on list_actions Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../src/action-flow-input-params.test.ts | 152 ++++++++++++++++++ .../src/mcp-list-actions-flow-params.test.ts | 116 +++++++++++++ 2 files changed, 268 insertions(+) create mode 100644 packages/runtime/src/action-flow-input-params.test.ts create mode 100644 packages/runtime/src/mcp-list-actions-flow-params.test.ts diff --git a/packages/runtime/src/action-flow-input-params.test.ts b/packages/runtime/src/action-flow-input-params.test.ts new file mode 100644 index 0000000000..5ac0ae48b3 --- /dev/null +++ b/packages/runtime/src/action-flow-input-params.test.ts @@ -0,0 +1,152 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `list_actions` publishes a FLOW action's input names (#15705). + * + * The MCP `list_actions` tool description promises each action's "input + * parameters", and `summarizeActionParams` delivered them by iterating + * `action.params` — the script-action declaration. A flow-typed action almost + * never declares `params`: its input contract is the target flow's `isInput` + * variables, which is what `seedDeclaredVariables` binds the caller's bag into. + * So every flow action listed with no `params` key at all, and an agent could + * see the action, could invoke it, and had no way to learn a single input name + * — the reported reproduction passed `due_date` where the flow declares + * `dueDate` because guessing was the only move available. + * + * Pinned here: + * + * 1. the inputs are published, in declaration order, with the collecting + * screen field's `label` / `type` / `required` / `options` folded in; + * 2. a variable that is NOT `isInput` stays private — the listing publishes an + * input contract, not the flow's internals; + * 3. an author's own `action.params` still WINS, so this can only fill a + * silence and no existing listing changes shape; + * 4. every absence is inert: no flow, a flow with no variables, a non-flow + * action — all answer exactly as they did before. + * + * `required` is read off the screen field alone. A flow variable has no + * `required` key, and inferring one from "declares no `defaultValue`" would + * invent a contract the author never wrote — the same reason the executor's + * satisfaction verdict one package over reads `required` from the field spec. + */ + +import { describe, it, expect } from 'vitest'; + +import { + summarizeAction, + summarizeActionParams, + summarizeFlowInputParams, +} from './action-execution.js'; + +const NO_DEPS: any = {}; + +/** The card's specimen: `schedule_followup`, `type: 'flow'`, no declared params. */ +const FLOW_ACTION = { + name: 'schedule_followup', + label: 'Schedule Follow-up', + type: 'flow', + target: 'schedule_followup', + ai: { exposed: true }, + locations: ['record_header'], +}; + +/** + * The target flow. `internal_cursor` is deliberately not an input, and + * `activityType` is collected on a SECOND screen — a wizard's later step is + * still part of the input contract. + */ +const FLOW = { + name: 'schedule_followup', + type: 'screen', + variables: [ + { name: 'subject', type: 'text', isInput: true }, + { name: 'dueDate', type: 'text', isInput: true }, + { name: 'activityType', type: 'text', isInput: true }, + { name: 'internal_cursor', type: 'number', isInput: false }, + ], + nodes: [ + { id: 'start', type: 'start' }, + { + id: 'screen_1', type: 'screen', + config: { + fields: [ + { name: 'subject', label: 'Subject', type: 'text', required: true }, + { name: 'dueDate', label: 'Due date', type: 'date', required: true }, + ], + }, + }, + { + id: 'screen_2', type: 'screen', + config: { + fields: [ + { + name: 'activityType', label: 'Activity type', type: 'select', + options: [{ value: 'call', label: 'Call' }, { value: 'email', label: 'Email' }], + }, + ], + }, + }, + { id: 'end', type: 'end' }, + ], +}; + +describe('summarizeFlowInputParams (#15705)', () => { + it('publishes every isInput variable, in declaration order, enriched from its screen field', () => { + expect(summarizeFlowInputParams(NO_DEPS, FLOW)).toEqual([ + { name: 'subject', type: 'string', required: true, description: 'Subject' }, + { name: 'dueDate', type: 'string', required: true, description: 'Due date' }, + { + name: 'activityType', type: 'string', required: false, + description: 'Activity type', enum: ['call', 'email'], + }, + ]); + }); + + it('keeps a non-input variable private', () => { + const names = summarizeFlowInputParams(NO_DEPS, FLOW).map((p) => p.name); + expect(names).not.toContain('internal_cursor'); + }); + + it('lists an input no screen collects — without the screen-only enrichments', () => { + const flow = { ...FLOW, variables: [{ name: 'silent', type: 'number', isInput: true }] }; + expect(summarizeFlowInputParams(NO_DEPS, flow)).toEqual([ + { name: 'silent', type: 'number', required: false }, + ]); + }); + + it('is inert on every absence', () => { + expect(summarizeFlowInputParams(NO_DEPS, undefined)).toEqual([]); + expect(summarizeFlowInputParams(NO_DEPS, {})).toEqual([]); + expect(summarizeFlowInputParams(NO_DEPS, { variables: [] })).toEqual([]); + expect(summarizeFlowInputParams(NO_DEPS, { variables: [{ name: 'x', isInput: false }] })).toEqual([]); + }); +}); + +describe('summarizeActionParams / summarizeAction fall back to the flow (#15705)', () => { + it('CONTROL — the reported shape: no flow resolved, no params key, exactly as before', () => { + expect(summarizeActionParams(NO_DEPS, FLOW_ACTION, undefined)).toEqual([]); + expect(summarizeAction(NO_DEPS, FLOW_ACTION, undefined, 'crm_lead')).not.toHaveProperty('params'); + }); + + it('surfaces the flow inputs once the flow is resolved', () => { + const summary = summarizeAction(NO_DEPS, FLOW_ACTION, undefined, 'crm_lead', FLOW); + expect(summary.params.map((p: any) => p.name)).toEqual(['subject', 'dueDate', 'activityType']); + // The rest of the summary is untouched by this change. + expect(summary).toMatchObject({ name: 'schedule_followup', type: 'flow', requiresRecord: true }); + }); + + it("CONTROL — an author's own declared params still win outright", () => { + const declaring = { + ...FLOW_ACTION, + params: [{ name: 'only_this', type: 'text', required: true, label: 'Only this' }], + }; + const params = summarizeActionParams(NO_DEPS, declaring, undefined, FLOW); + expect(params.map((p: any) => p.name)).toEqual(['only_this']); + }); + + it('CONTROL — a non-flow action handed a flow is unchanged (the caller resolves none)', () => { + const script = { name: 'close_case', type: 'script', target: 'closeCase', locations: ['record_header'] }; + expect(summarizeActionParams(NO_DEPS, script, undefined)).toEqual([]); + expect(summarizeAction(NO_DEPS, script, undefined, 'crm_case')).not.toHaveProperty('params'); + }); +}); diff --git a/packages/runtime/src/mcp-list-actions-flow-params.test.ts b/packages/runtime/src/mcp-list-actions-flow-params.test.ts new file mode 100644 index 0000000000..88b08a9573 --- /dev/null +++ b/packages/runtime/src/mcp-list-actions-flow-params.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The WIRE half of #15705: `list_actions` asks the automation service for the + * flow behind a flow action, so the input names actually reach the agent. + * + * `summarizeActionParams`' own pins (`action-flow-input-params.test.ts`) prove + * the projection. They cannot prove anybody performs it: the bridge held only + * `Boolean(automationService)` and passed the summary no flow at all, so the + * projection would have sat unreachable behind a green unit suite — the exact + * "declared but nothing calls it" shape. This file pins the call. + * + * `getFlow` is OPTIONAL on `IAutomationService`, so the degradation is pinned + * too: a service that does not implement it, and a flow the registry does not + * hold, both answer exactly as the listing answered before this change. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { HttpDispatcher } from './http-dispatcher.js'; + +/** The card's specimen, as an object-embedded declaration. */ +const FLOW_ACTION = { + name: 'schedule_followup', + label: 'Schedule Follow-up', + objectName: 'crm_lead', + type: 'flow', + target: 'schedule_followup', + locations: ['record_header'], + ai: { exposed: true, description: 'Schedule a follow-up task for this lead.' }, +}; + +const FLOW = { + name: 'schedule_followup', + type: 'screen', + variables: [ + { name: 'subject', type: 'text', isInput: true }, + { name: 'dueDate', type: 'text', isInput: true }, + ], + nodes: [ + { id: 'start', type: 'start' }, + { + id: 'screen_1', type: 'screen', + config: { + fields: [ + { name: 'subject', label: 'Subject', type: 'text', required: true }, + { name: 'dueDate', label: 'Due date', type: 'date', required: true }, + ], + }, + }, + ], +}; + +function makeBridge(automation: any) { + const object = { name: 'crm_lead', label: 'Lead', fields: {}, actions: [FLOW_ACTION] }; + const ql: any = { + executeAction: vi.fn(), + registry: { getObject: () => object }, + find: vi.fn(async () => []), + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + }; + const metadata: any = { + listObjects: vi.fn(async () => [object]), + getObject: vi.fn(async () => object), + }; + const kernel: any = { + context: { + getService: (n: string) => + n === 'objectql' || n === 'data' ? ql : n === 'metadata' ? metadata : n === 'automation' ? automation : null, + }, + }; + const dispatcher = new HttpDispatcher(kernel); + const ctx: any = { + request: {}, environmentId: 'platform', + executionContext: { userId: 'u1', systemPermissions: [] }, + }; + return (dispatcher as any).buildMcpBridge(ctx); +} + +async function listed(automation: any) { + const actions = await makeBridge(automation).listActions(); + return actions.find((a: any) => a.name === 'schedule_followup'); +} + +describe('list_actions surfaces a flow action’s inputs (#15705)', () => { + it('asks getFlow for the action’s target and publishes the flow’s inputs as params', async () => { + const getFlow = vi.fn(async () => FLOW); + const summary = await listed({ execute: vi.fn(), getFlow }); + expect(getFlow).toHaveBeenCalledWith('schedule_followup'); + expect(summary.params).toEqual([ + { name: 'subject', type: 'string', required: true, description: 'Subject' }, + { name: 'dueDate', type: 'string', required: true, description: 'Due date' }, + ]); + }); + + it('CONTROL — the reported shape: a service with no getFlow lists exactly as before', async () => { + const summary = await listed({ execute: vi.fn() }); + expect(summary).toBeDefined(); + expect(summary).not.toHaveProperty('params'); + expect(summary).toMatchObject({ name: 'schedule_followup', type: 'flow', requiresRecord: true }); + }); + + it('CONTROL — a target the registry does not hold degrades to the same shape', async () => { + const summary = await listed({ execute: vi.fn(), getFlow: vi.fn(async () => null) }); + expect(summary).not.toHaveProperty('params'); + }); + + it('CONTROL — a getFlow that THROWS does not fail the listing', async () => { + const summary = await listed({ + execute: vi.fn(), + getFlow: vi.fn(async () => { throw new Error('registry unavailable'); }), + }); + expect(summary).toBeDefined(); + expect(summary).not.toHaveProperty('params'); + }); +}); From 3bd7f54b0967e454603033f322ee80fbf082469d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:19:26 +0000 Subject: [PATCH 4/9] docs(#15705): document headless screen satisfaction + flow input params Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../screen-flow-headless-satisfaction.md | 17 +++++++ content/docs/automation/flows.mdx | 51 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 .changeset/screen-flow-headless-satisfaction.md diff --git a/.changeset/screen-flow-headless-satisfaction.md b/.changeset/screen-flow-headless-satisfaction.md new file mode 100644 index 0000000000..f143cf149e --- /dev/null +++ b/.changeset/screen-flow-headless-satisfaction.md @@ -0,0 +1,17 @@ +--- +"@objectstack/service-automation": minor +"@objectstack/runtime": minor +--- + +A screen flow can now be completed by a headless caller, and `list_actions` publishes its input names. + +An `ai.exposed` action whose target is a **screen flow** could be started over MCP and never finished. `run_action` seeded the flow's `isInput` variables from the caller's `params` — correctly — and the screen node suspended anyway, because the only inputs to that decision were "does the node declare fields" and the author's `waitForInput` flag. The MCP tool set has no verb to resume a parked run, so `ai.exposed` meant "the agent can invoke this", not "the agent can complete this". The fallback an agent took instead — re-implementing the flow's tail with `create_record` + `update_record` — bypasses whatever business rules the flow encapsulated. + +Two independent halves: + +- **A screen the caller already answered no longer pauses.** When the caller named at least one of the screen's own fields and every `required` one has a value from that caller, there is nothing left to collect and the run continues. Optional fields may come from anywhere (including a declared `defaultValue`). +- **`list_actions` publishes a flow action's inputs.** A `type: 'flow'` action's contract is its target flow's `isInput` variables, not `action.params`; those are now surfaced in declaration order with the `label`, `type`, `required` and select `options` of the screen field that collects each one. An action that declares its own `params[]` keeps them — the flow is read only where the action declared nothing. + +**Interactive runs are unchanged.** A console run supplies none of the screen's fields, so it renders the form exactly as before — including when the subject record carries a column named like one of the screen's fields, which is the trigger record speaking rather than the caller. Two screens never take the new path, because they declare nothing to satisfy and must not be answered vacuously: a message-only screen (no fields), and any screen whose author wrote `waitForInput: true`. `waitForInput: false` remains the wrong tool for the headless case — it skips the form for interactive users too. + +⚠️ This does **not** make every screen flow completable over MCP. A call that omits the inputs still parks, and nothing on that surface can resume it; that half is a resume verb and is not this change. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index dddfa96ac9..9fda7bee2b 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -734,6 +734,57 @@ producer), and `'service'` when the decision to continue belongs to your service rather than to whoever holds the run id. Declaring neither leaves your pauses refused on the generic route, and the engine warns about it at registration. +### A screen the caller already answered does not pause + +A `screen` node exists to collect input. When the run's **caller already supplied +it**, there is nothing left to collect, so the run continues past the screen +instead of suspending. + +That is what makes a screen flow reachable from a headless invoker. An +`ai.exposed` action whose target is a screen flow is started with `params`; those +bind to the flow's `isInput` variables, and before this the screen suspended +anyway — returning a form to a caller with no way to submit one, so the run +parked forever. + +The screen is treated as answered only when **all** of these hold: + +- the caller named **at least one of this screen's own fields** — a run that + named none of them is not driving the screen, and it renders as always; +- every `required` field has a value (an empty or blank string is no value), + supplied by that caller. A value that came from the trigger record, from an + earlier node or from a declared `defaultValue` does not answer a `required` + field; optional fields may come from anywhere; +- the field has no `visibleWhen` the caller left unanswered. The server cannot + evaluate that predicate — it has no rendered form and no collected values — so + a conditional `required` field the caller did not name keeps the screen + interactive. (The resume door makes the opposite call for the opposite reason: + there, enforcing a hidden field's `required` would dead-end a run at Submit.) + +Two screens never take this path, because they declare nothing to satisfy and a +bag must not answer them vacuously: a **message-only** screen (no `fields`), and +any screen whose author wrote `waitForInput: true` — that flag is an explicit +"show this", and a confirmation step is not something a params bag may skip. + +**Interactive runs are unaffected.** A console run supplies none of the screen's +fields, so it renders the form exactly as before — including when the subject +record happens to carry a column named like one of them, which is not the caller +speaking. ⛔ `waitForInput: false` remains the wrong tool for the headless case: +it skips the form for interactive users too. + +⚠️ This does **not** make every screen flow completable from a headless caller. A +call that omits the inputs still parks, and the MCP tool set has no verb to +resume a parked run. + +### `list_actions` publishes a flow action's inputs + +An action's `params` in the MCP action listing come from its declared +`params[]`. A `type: 'flow'` action rarely declares any — its input contract is +the **target flow's `isInput` variables**, which is what the caller's `params` +bag binds into. Those are published instead, in declaration order, carrying the +`label`, `type`, `required` and select `options` of the screen field that +collects each one. An action that declares its own `params[]` keeps them: the +flow is read only when the action itself declares nothing. + ### Parallel approvals — one aggregating node, not two pauses "Finance **and** legal must both sign off, concurrently" is **one `approval` From 5e21be2ded57f73f5ee01aa884f1dac2be31385f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 09:01:15 +0000 Subject: [PATCH 5/9] fix(automation): the dispatcher's row-id seeds are not the caller speaking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both dispatch doors put the launched row's id into params under names that are not record columns — recordId and the camelCase object-id alias on the trigger door (which sets no context.record at all), plus the action's declared recordIdParam on the actions door. A screen field named like one of them read as caller-supplied on a run that supplied nothing, so an interactive console launch could skip the screen. Refuse the two derivable names outright and the third by value (identical to the row id). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../screen-headless-satisfaction.test.ts | 113 ++++++++++++++++++ .../src/screen-input-contract.ts | 48 +++++++- 2 files changed, 155 insertions(+), 6 deletions(-) diff --git a/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts index 331556df09..28dca04dcb 100644 --- a/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts +++ b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts @@ -108,6 +108,27 @@ function actionContext( } as AutomationContext; } +/** + * The OTHER door, and the one a record-only provenance leg cannot see: + * `POST /api/v1/automation/:name/trigger`. `buildAutomationContext` + * (`@objectstack/runtime`) turns the console's `{recordId, objectName, params}` + * into `params.recordId` PLUS the camelCase `Id` alias — and sets + * **no `context.record` at all**. So neither of those two keys is a column, + * nothing can disprove them from the record, and a screen field named like + * either of them would read as "the caller supplied this" on an interactive + * console launch that supplied nothing. + */ +function triggerDoorContext( + recordId: string, + params: Record = {}, +): AutomationContext { + return { + object: 'crm_lead', + event: 'manual', + params: { ...params, recordId, crmLeadId: recordId }, + } as AutomationContext; +} + const LEAD = { id: 'lead_1', name: 'Acme', company: 'Acme Inc' }; describe('screen headless satisfaction (#15705)', () => { @@ -209,6 +230,98 @@ describe('screen headless satisfaction (#15705)', () => { expect(res.status).toBe('paused'); }); + // ── The row-id seeds are not the caller speaking ────────────────────── + // + // Both dispatch doors put the launched row's id into `params` under names + // that are NOT record columns, so the record leg alone cannot disprove + // them. A screen field named like one of them must therefore not count as + // answered — otherwise an interactive console launch, which supplies + // nothing but the record it was launched from, skips the screen. + + it('CONTROL — trigger door: a required `recordId` field does NOT satisfy an interactive launch', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [ + { name: 'recordId', label: 'Record', type: 'text', required: true }, + { name: 'notes', label: 'Notes', type: 'text' }, + ]; + flow.variables = [ + { name: 'recordId', type: 'text', isInput: true, isOutput: true }, + { name: 'notes', type: 'text', isInput: true, isOutput: true }, + ]; + register({}, flow); + const res = await engine.execute('schedule_followup', triggerDoorContext('lead_1')); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + }); + + it('CONTROL — trigger door: the camelCase `Id` alias does not satisfy it either', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'crmLeadId', label: 'Lead', type: 'text', required: true }]; + flow.variables = [{ name: 'crmLeadId', type: 'text', isInput: true, isOutput: true }]; + register({}, flow); + const res = await engine.execute('schedule_followup', triggerDoorContext('lead_1')); + expect(res.status).toBe('paused'); + }); + + it('CONTROL — actions door: the same two seeded id keys do not satisfy it', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [ + { name: 'recordId', label: 'Record', type: 'text', required: true }, + { name: 'crmLeadId', label: 'Lead', type: 'text', required: true }, + ]; + flow.variables = [ + { name: 'recordId', type: 'text', isInput: true, isOutput: true }, + { name: 'crmLeadId', type: 'text', isInput: true, isOutput: true }, + ]; + register({}, flow); + const res = await engine.execute('schedule_followup', actionContext(LEAD, {})); + expect(res.status).toBe('paused'); + }); + + /** + * `recordIdParam` is action-level metadata the executor cannot see, so its + * NAME cannot be refused — its VALUE is. The dispatcher seeds it with the + * row id, so a field bound to the row id is never the caller speaking. + */ + it("CONTROL — a field carrying the row id under the action's own `recordIdParam` name does not satisfy it", async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'leadRef', label: 'Lead ref', type: 'text', required: true }]; + flow.variables = [{ name: 'leadRef', type: 'text', isInput: true, isOutput: true }]; + register({}, flow); + // What `seedFlowActionParams` produces for `recordIdParam: 'leadRef'`. + const res = await engine.execute('schedule_followup', actionContext(LEAD, { leadRef: LEAD.id })); + expect(res.status).toBe('paused'); + }); + + it('trigger door: a genuine caller param still satisfies the screen', async () => { + register(); + const res = await engine.execute('schedule_followup', triggerDoorContext('lead_1', { + subject: 'Call Acme back', dueDate: '2026-09-09', + })); + expect(res.status).not.toBe('paused'); + expect(res.output).toMatchObject({ subject: 'Call Acme back', dueDate: '2026-09-09' }); + }); + + /** + * The record-change trigger's shape, pinned for the MECHANISM as well as + * the outcome: it sets `params` to the SAME object it sets as `record` + * (`record-change-trigger.ts`), so `params` is emphatically NOT empty — it + * pauses because every key is identity-equal to the record's own value, not + * because there was nothing to read. + */ + it('CONTROL — record-change trigger shape: params IS the record, and it still pauses', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'company', label: 'Company', type: 'text', required: true }]; + flow.variables = [{ name: 'company', type: 'text', isInput: true, isOutput: true }]; + register({}, flow); + const isolated = { ...LEAD }; + const res = await engine.execute('schedule_followup', { + record: isolated, params: isolated, object: 'crm_lead', event: 'on_update', + } as AutomationContext); + expect(res.status).toBe('paused'); + expect(Object.keys((isolated as Record))).toContain('company'); + }); + // ── Vacuity guards: a screen with nothing to satisfy must not be skipped ── it('CONTROL — an explicit `waitForInput: true` still pauses even when fully supplied', async () => { diff --git a/packages/services/service-automation/src/screen-input-contract.ts b/packages/services/service-automation/src/screen-input-contract.ts index a988f828aa..ab912288c9 100644 --- a/packages/services/service-automation/src/screen-input-contract.ts +++ b/packages/services/service-automation/src/screen-input-contract.ts @@ -179,26 +179,60 @@ const NOTHING_SUPPLIED: HeadlessScreenVerdict = { satisfied: false, supplied: [] * run — which supplies nothing — skip a screen whose field happens to share a * name with a column of the record it was launched from. * - * Two legs, either of which proves caller provenance: + * The record is one seed. **The row ID is the other**, and it is the one a + * record leg alone cannot see: both doors put the launched row's id into + * `params` under names that are NOT columns — `recordId` and the camelCase + * `Id` alias on the trigger door (`buildAutomationContext`, which sets + * NO `context.record` at all), plus the action's declared `recordIdParam` on + * the actions door (`seedFlowActionParams`). A screen field named like any of + * them would otherwise read as caller-supplied on a run that supplied nothing, + * and an interactive console launch would skip the screen. So those are + * refused up front, by NAME for the two the executor can derive and by VALUE + * (identical to the row id) for the third, whose name is action-level metadata + * this executor cannot see. + * + * What remains proves caller provenance: * * - the record has no such key at all ⇒ the record leg cannot be the source; * - the record HAS the key but `params` holds a different value ⇒ the - * caller's bag overwrote it. `{ ...record }` copies the record's own value + * caller's bag overwrote it. The record spread copies the record's own value * by reference/primitive, so a run that supplied nothing is `Object.is`-equal - * here, always. Equality is therefore "indistinguishable", not "caller-set". + * here. Equality is therefore "indistinguishable", not "caller-set". * * The ambiguous case (same key, same value) resolves to NOT caller-supplied, * which costs a headless run a pause it might have been allowed to skip and * costs an interactive run nothing. That asymmetry is deliberate: every * uncertainty in this module must land on today's behaviour. + * + * ⚠️ **The identity leg is weaker across a durable resume.** A suspended run + * persists its `context` as JSON (`suspended-run-store.ts`), so a run continued + * from the store judges against a `JSON.parse`d copy: a NON-primitive column + * value (an array, an object) is no longer `Object.is`-equal to the one in + * `params`, and a later wizard screen colliding with such a column can read as + * caller-supplied. Primitive columns are unaffected. Stated, not fixed here — + * the remedy is value comparison rather than identity, which is a different + * change and has its own card. */ function callerSupplied( name: string, - context: { params?: Record; record?: Record } | undefined, + context: { params?: Record; record?: Record; object?: string } | undefined, ): boolean { const params = context?.params; if (!params || params[name] === undefined) return false; + // Row-id seeds first: neither leg below can disprove them, because the + // trigger door sets no record and none of these names is a column. + if (name === 'recordId') return false; + const objectName = typeof context?.object === 'string' ? context.object.trim() : ''; + // The camelCase alias both doors seed, derived the same way they derive it. + if (objectName && name === `${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`) { + return false; + } const record = context?.record; + // The action's declared `recordIdParam` may seed a THIRD name this executor + // cannot know, always with the row id as its value — so the value is what + // refuses it. A caller who genuinely sends the row id as a screen value only + // loses the skip, which is this module's standing failure direction. + if (record?.id !== undefined && Object.is(params[name], record.id)) return false; if (!record || !Object.prototype.hasOwnProperty.call(record, name)) return true; return !Object.is(params[name], record[name]); } @@ -218,7 +252,9 @@ function callerSupplied( * the verdict is `false` the moment any of them is unproven: * * 1. **The caller supplied at least one of THIS screen's declared fields** - * ({@link callerSupplied}). Without this leg a screen whose fields are all + * ({@link callerSupplied}) — which refuses the row-id keys both dispatch + * doors seed, so a launch that carried only a `recordId` has supplied + * nothing. Without this leg a screen whose fields are all * optional would be vacuously "satisfied" and would stop rendering for * everyone — the loudest way to break the interactive path. A run that * named none of this screen's fields is not driving it, so it pauses. @@ -243,7 +279,7 @@ function callerSupplied( export function judgeHeadlessScreen( fields: readonly ScreenFieldContract[], variables: ReadonlyMap, - context: { params?: Record; record?: Record } | undefined, + context: { params?: Record; record?: Record; object?: string } | undefined, ): HeadlessScreenVerdict { const declared = fields.filter((f) => typeof f?.name === 'string' && f.name.length > 0); if (declared.length === 0) return NOTHING_SUPPLIED; From 60fdd313855e4a8b7ef60a862d4a8972082e5e34 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 09:03:17 +0000 Subject: [PATCH 6/9] docs(#15705): correct three claims the console-door finding falsified The trigger door seeds recordId and the camelCase object-id alias into params and sets no context.record, so 'a console run supplies none of the screen's fields' was not true as written; and a record-change trigger sets params to the SAME object as record, so it pauses on the identity leg rather than on absent params. Also states the durable-resume gap: a JSON-rehydrated context loses the identity a non-scalar column's comparison relies on. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../screen-flow-headless-satisfaction.md | 4 +++- content/docs/automation/flows.mdx | 23 ++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/.changeset/screen-flow-headless-satisfaction.md b/.changeset/screen-flow-headless-satisfaction.md index f143cf149e..563f640186 100644 --- a/.changeset/screen-flow-headless-satisfaction.md +++ b/.changeset/screen-flow-headless-satisfaction.md @@ -12,6 +12,8 @@ Two independent halves: - **A screen the caller already answered no longer pauses.** When the caller named at least one of the screen's own fields and every `required` one has a value from that caller, there is nothing left to collect and the run continues. Optional fields may come from anywhere (including a declared `defaultValue`). - **`list_actions` publishes a flow action's inputs.** A `type: 'flow'` action's contract is its target flow's `isInput` variables, not `action.params`; those are now surfaced in declaration order with the `label`, `type`, `required` and select `options` of the screen field that collects each one. An action that declares its own `params[]` keeps them — the flow is read only where the action declared nothing. -**Interactive runs are unchanged.** A console run supplies none of the screen's fields, so it renders the form exactly as before — including when the subject record carries a column named like one of the screen's fields, which is the trigger record speaking rather than the caller. Two screens never take the new path, because they declare nothing to satisfy and must not be answered vacuously: a message-only screen (no fields), and any screen whose author wrote `waitForInput: true`. `waitForInput: false` remains the wrong tool for the headless case — it skips the form for interactive users too. +**Interactive runs are unchanged.** A console launch carries the record it was launched from and that record's id — never a value for the screen's own fields — so the form renders exactly as before. That covers both shapes a launch actually supplies: a subject-record column named like one of the screen's fields, and a field named like one of the row-id keys the dispatch doors seed (`recordId`, the camelCase `Id` alias, an action's declared `recordIdParam`), none of which counts as the caller answering the screen. Two screens never take the new path, because they declare nothing to satisfy and must not be answered vacuously: a message-only screen (no fields), and any screen whose author wrote `waitForInput: true`. `waitForInput: false` remains the wrong tool for the headless case — it skips the form for interactive users too. + +⚠️ One known gap, on the trigger-record leg only: a run continued from the **durable** suspended-run store judges against a JSON copy of its context, so a later wizard screen whose field collides with a **non-scalar** column (an array or object) of the trigger record can read as caller-supplied and be skipped. Scalar columns are unaffected, as is any run that has not been through a pause. ⚠️ This does **not** make every screen flow completable over MCP. A call that omits the inputs still parks, and nothing on that surface can resume it; that half is a resume verb and is not this change. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 9fda7bee2b..9990088dec 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -749,7 +749,11 @@ parked forever. The screen is treated as answered only when **all** of these hold: - the caller named **at least one of this screen's own fields** — a run that - named none of them is not driving the screen, and it renders as always; + named none of them is not driving the screen, and it renders as always. The + row id a launch carries does **not** count as naming a field: both dispatch + doors seed it into the params bag under `recordId` and the camelCase + `Id` alias (and under an action's declared `recordIdParam`), and none + of those is the caller supplying a screen value; - every `required` field has a value (an empty or blank string is no value), supplied by that caller. A value that came from the trigger record, from an earlier node or from a declared `defaultValue` does not answer a `required` @@ -765,11 +769,18 @@ bag must not answer them vacuously: a **message-only** screen (no `fields`), and any screen whose author wrote `waitForInput: true` — that flag is an explicit "show this", and a confirmation step is not something a params bag may skip. -**Interactive runs are unaffected.** A console run supplies none of the screen's -fields, so it renders the form exactly as before — including when the subject -record happens to carry a column named like one of them, which is not the caller -speaking. ⛔ `waitForInput: false` remains the wrong tool for the headless case: -it skips the form for interactive users too. +**Interactive runs are unaffected.** A console launch carries the record it was +launched from and that record's id — never a value for the screen's own fields — +so the form renders exactly as before, including when the subject record carries +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. + +⚠️ One known gap, on the trigger-record leg only: a run continued from the +**durable** suspended-run store judges against a JSON copy of its context, so a +later wizard screen whose field collides with a **non-scalar** column (an array +or object) of the trigger record can read as caller-supplied. Scalar columns are +unaffected, as is any run that has not been through a pause. ⚠️ This does **not** make every screen flow completable from a headless caller. A call that omits the inputs still parks, and the MCP tool set has no verb to From 6fdffb74ab98078952e74fab050dd30d730a6f0b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 09:57:05 +0000 Subject: [PATCH 7/9] fix(automation): recover the seeded row id from the bag, not just record.id A non-default recordIdField makes the seeded row id a column other than id, so an action's declared recordIdParam gave that value a third key the record does not carry: not a column, not record.id, not a derivable name. It read as caller-supplied and the screen was SKIPPED, not paused, on a launch that supplied nothing. The dispatcher seeds the same row id under every id key it knows, so params.recordId (and the camelCase alias) recover it without knowing the action-level name. Also states the accepted cost: a screen field named recordId or the object-id alias is always collected interactively. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../screen-flow-headless-satisfaction.md | 2 +- content/docs/automation/flows.mdx | 6 ++- .../screen-headless-satisfaction.test.ts | 34 +++++++++++++++++ .../src/screen-input-contract.ts | 37 ++++++++++++++----- 4 files changed, 67 insertions(+), 12 deletions(-) diff --git a/.changeset/screen-flow-headless-satisfaction.md b/.changeset/screen-flow-headless-satisfaction.md index 563f640186..fd0c9ad346 100644 --- a/.changeset/screen-flow-headless-satisfaction.md +++ b/.changeset/screen-flow-headless-satisfaction.md @@ -12,7 +12,7 @@ Two independent halves: - **A screen the caller already answered no longer pauses.** When the caller named at least one of the screen's own fields and every `required` one has a value from that caller, there is nothing left to collect and the run continues. Optional fields may come from anywhere (including a declared `defaultValue`). - **`list_actions` publishes a flow action's inputs.** A `type: 'flow'` action's contract is its target flow's `isInput` variables, not `action.params`; those are now surfaced in declaration order with the `label`, `type`, `required` and select `options` of the screen field that collects each one. An action that declares its own `params[]` keeps them — the flow is read only where the action declared nothing. -**Interactive runs are unchanged.** A console launch carries the record it was launched from and that record's id — never a value for the screen's own fields — so the form renders exactly as before. That covers both shapes a launch actually supplies: a subject-record column named like one of the screen's fields, and a field named like one of the row-id keys the dispatch doors seed (`recordId`, the camelCase `Id` alias, an action's declared `recordIdParam`), none of which counts as the caller answering the screen. Two screens never take the new path, because they declare nothing to satisfy and must not be answered vacuously: a message-only screen (no fields), and any screen whose author wrote `waitForInput: true`. `waitForInput: false` remains the wrong tool for the headless case — it skips the form for interactive users too. +**Interactive runs are unchanged.** A console launch carries the record it was launched from and that record's id — never a value for the screen's own fields — so the form renders exactly as before. That covers both shapes a launch actually supplies: a subject-record column named like one of the screen's fields, and a field named like one of the row-id keys the dispatch doors seed (`recordId`, the camelCase `Id` alias, an action's declared `recordIdParam`), none of which counts as the caller answering the screen — the two fixed names outright, and any value equal to the launched row's id whatever key carries it. One accepted cost of that: **a screen field named `recordId` or `Id` is always collected interactively**, even from a headless caller. Two screens never take the new path, because they declare nothing to satisfy and must not be answered vacuously: a message-only screen (no fields), and any screen whose author wrote `waitForInput: true`. `waitForInput: false` remains the wrong tool for the headless case — it skips the form for interactive users too. ⚠️ One known gap, on the trigger-record leg only: a run continued from the **durable** suspended-run store judges against a JSON copy of its context, so a later wizard screen whose field collides with a **non-scalar** column (an array or object) of the trigger record can read as caller-supplied and be skipped. Scalar columns are unaffected, as is any run that has not been through a pause. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 9990088dec..c93926ad40 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -753,7 +753,11 @@ The screen is treated as answered only when **all** of these hold: row id a launch carries does **not** count as naming a field: both dispatch doors seed it into the params bag under `recordId` and the camelCase `Id` alias (and under an action's declared `recordIdParam`), and none - of those is the caller supplying a screen value; + of those is the caller supplying a screen value. The two fixed names are + refused outright, and any value equal to the launched row's id is refused + whatever key carries it — so **a screen field named `recordId` or `Id` + is always collected interactively**, even from a headless caller. Name a field + you want a headless caller to fill something else; - every `required` field has a value (an empty or blank string is no value), supplied by that caller. A value that came from the trigger record, from an earlier node or from a declared `defaultValue` does not answer a `required` diff --git a/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts index 28dca04dcb..9d82a74f76 100644 --- a/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts +++ b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts @@ -293,6 +293,40 @@ describe('screen headless satisfaction (#15705)', () => { expect(res.status).toBe('paused'); }); + /** + * The shape review round 2 drove, and the one this file's author had + * characterised as pause-only when it in fact SKIPPED. + * + * `recordIdField: 'token'` makes the seeded row id the record's `token` + * column rather than its `id`, and `recordIdParam: 'sessionToken'` gives + * that value a THIRD key the record does not carry. So: not a column (the + * record leg cannot see it), not `record.id` (the row-id value leg, as + * first written, could not see it either), and not a derivable name. It + * read as caller-supplied on a launch that supplied nothing, and the run + * completed with `{ sessionToken: 'tok_9' }`. + * + * What closes it: the dispatcher seeds the SAME row id under every one of + * its id keys, so `params.recordId` still carries it and the value is + * recoverable from the bag itself, without knowing the action-level name. + */ + it('CONTROL — a non-id `recordIdField` seeded under a third `recordIdParam` name does not satisfy it', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'sessionToken', label: 'Session', type: 'text', required: true }]; + flow.variables = [{ name: 'sessionToken', type: 'text', isInput: true, isOutput: true }]; + register({}, flow); + // The authentic `seedFlowActionParams` bag for + // `recordIdField: 'token'` + `recordIdParam: 'sessionToken'`: the row + // id is `record.token`, seeded under all three id keys. + const session = { id: 'sess_1', token: 'tok_9', label: 'Web session' }; + const res = await engine.execute('schedule_followup', { + record: session, + object: 'crm_lead', + params: { ...session, recordId: 'tok_9', crmLeadId: 'tok_9', sessionToken: 'tok_9' }, + } as AutomationContext); + expect(res.status).toBe('paused'); + expect(res.screen?.nodeId).toBe('screen_1'); + }); + it('trigger door: a genuine caller param still satisfies the screen', async () => { register(); const res = await engine.execute('schedule_followup', triggerDoorContext('lead_1', { diff --git a/packages/services/service-automation/src/screen-input-contract.ts b/packages/services/service-automation/src/screen-input-contract.ts index ab912288c9..34585dec87 100644 --- a/packages/services/service-automation/src/screen-input-contract.ts +++ b/packages/services/service-automation/src/screen-input-contract.ts @@ -221,20 +221,37 @@ function callerSupplied( if (!params || params[name] === undefined) return false; // Row-id seeds first: neither leg below can disprove them, because the // trigger door sets no record and none of these names is a column. - if (name === 'recordId') return false; const objectName = typeof context?.object === 'string' ? context.object.trim() : ''; // The camelCase alias both doors seed, derived the same way they derive it. - if (objectName && name === `${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`) { - return false; - } + const aliasKey = objectName + ? `${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id` + : undefined; + if (name === 'recordId' || (aliasKey !== undefined && name === aliasKey)) return false; + const record = context?.record; - // The action's declared `recordIdParam` may seed a THIRD name this executor - // cannot know, always with the row id as its value — so the value is what - // refuses it. A caller who genuinely sends the row id as a screen value only - // loses the skip, which is this module's standing failure direction. - if (record?.id !== undefined && Object.is(params[name], record.id)) return false; + const value = params[name]; + // The action's declared `recordIdParam` seeds a THIRD name this executor + // cannot know — action-level metadata is not on the context — so its VALUE + // is what refuses it. The dispatcher seeds the SAME row id under every id + // key it knows, which makes the id recoverable from the bag itself: + // + // - `params.recordId`, always seeded, and the only candidate that survives + // a NON-DEFAULT `recordIdField`. That case is why this leg exists: with + // `recordIdField: 'token'` the row id is `record.token`, so comparing + // against `record.id` alone missed it and the screen was SKIPPED, not + // paused — measured, then pinned. + // - the camelCase alias's value, seeded the same way, as a second reading + // of the same id for the case where a record column shadows `recordId`; + // - `record.id`, which covers the record-bearing doors directly. + // + // A caller who genuinely sends the row id as a screen value only loses the + // skip, which is this module's standing failure direction. + const seededRowIds: unknown[] = [params.recordId, record?.id]; + if (aliasKey !== undefined) seededRowIds.push(params[aliasKey]); + if (seededRowIds.some((id) => id !== undefined && Object.is(value, id))) return false; + if (!record || !Object.prototype.hasOwnProperty.call(record, name)) return true; - return !Object.is(params[name], record[name]); + return !Object.is(value, record[name]); } /** From 0f2416bdf58bd6fe272601e11918ebb25698fb93 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 09:58:46 +0000 Subject: [PATCH 8/9] test(#15705): make each row-id candidate separately load-bearing The token pin is satisfied by any of the three row-id candidates, so on its own it pinned the leg but not its parts: dropping params.recordId alone left all 21 green. Three fixtures now give each candidate a case only it can answer, by letting a record column shadow the other seed keys. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../screen-headless-satisfaction.test.ts | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts index 9d82a74f76..a050accb5b 100644 --- a/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts +++ b/packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts @@ -327,6 +327,62 @@ describe('screen headless satisfaction (#15705)', () => { expect(res.screen?.nodeId).toBe('screen_1'); }); + /** + * The row-id VALUE leg reads three candidates, and the `token` pin above is + * satisfied by any of them — so on its own it pins the leg but not its + * parts, and a future edit could delete one candidate and stay green + * (measured: dropping `params.recordId` alone left all 21 green). + * + * These three fixtures give each candidate a case only it can answer, by + * letting a record COLUMN shadow the other seed keys — which is the only + * way a seeded key stops carrying the row id, since `seedFlowActionParams` + * writes a key only `if (seeded[key] === undefined)` and the record spread + * came first. + */ + it('only `params.recordId` can refuse this one — the record shadows the alias key', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'sessionToken', label: 'Session', type: 'text', required: true }]; + flow.variables = [{ name: 'sessionToken', type: 'text', isInput: true, isOutput: true }]; + register({}, flow); + // recordIdField 'token' -> row id 'tok_9'; the record's own `crmLeadId` + // column survives the alias seed, so that candidate reads 'other_1'. + const record = { id: 'sess_1', token: 'tok_9', crmLeadId: 'other_1' }; + const res = await engine.execute('schedule_followup', { + record, object: 'crm_lead', + params: { ...record, recordId: 'tok_9', sessionToken: 'tok_9' }, + } as AutomationContext); + expect(res.status).toBe('paused'); + }); + + it('only the alias VALUE can refuse this one — the record shadows `recordId`', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'sessionToken', label: 'Session', type: 'text', required: true }]; + flow.variables = [{ name: 'sessionToken', type: 'text', isInput: true, isOutput: true }]; + register({}, flow); + const record = { id: 'sess_1', token: 'tok_9', recordId: 'shadow_1' }; + const res = await engine.execute('schedule_followup', { + record, object: 'crm_lead', + params: { ...record, crmLeadId: 'tok_9', sessionToken: 'tok_9' }, + } as AutomationContext); + expect(res.status).toBe('paused'); + }); + + it('only `record.id` can refuse this one — object-less action, and the record shadows `recordId`', async () => { + const flow: any = followupFlow(); + flow.nodes[1].config.fields = [{ name: 'subjectRef', label: 'Subject', type: 'text', required: true }]; + flow.variables = [{ name: 'subjectRef', type: 'text', isInput: true, isOutput: true }]; + register({}, flow); + // No `object` on the context (an object-less action), so no alias key + // is derivable at all; the record's own `recordId` column shadows the + // other seed. The default `recordIdField` leaves the row id at `id`. + const record = { id: 'lead_1', recordId: 'shadow_1' }; + const res = await engine.execute('schedule_followup', { + record, + params: { ...record, subjectRef: 'lead_1' }, + } as AutomationContext); + expect(res.status).toBe('paused'); + }); + it('trigger door: a genuine caller param still satisfies the screen', async () => { register(); const res = await engine.execute('schedule_followup', triggerDoorContext('lead_1', { From 2041951a18e94ada28aeb46b81d0297b0e22d72c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 13:09:40 +0000 Subject: [PATCH 9/9] docs(#15705): state the accepted cost precisely, in the reviewer's wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sentence was imprecise both ways: overstated, because an OPTIONAL row-id valued field is not collected interactively — it continues with the value bound, it simply does not count toward the caller-named condition; and understated, because values equal to a caller-overridden recordId or alias, or to a shadowing recordId column, are refused too. Replaced in both shipped places with one wording, identical in each. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/screen-flow-headless-satisfaction.md | 4 +++- content/docs/automation/flows.mdx | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.changeset/screen-flow-headless-satisfaction.md b/.changeset/screen-flow-headless-satisfaction.md index fd0c9ad346..eb138b9d93 100644 --- a/.changeset/screen-flow-headless-satisfaction.md +++ b/.changeset/screen-flow-headless-satisfaction.md @@ -12,7 +12,9 @@ Two independent halves: - **A screen the caller already answered no longer pauses.** When the caller named at least one of the screen's own fields and every `required` one has a value from that caller, there is nothing left to collect and the run continues. Optional fields may come from anywhere (including a declared `defaultValue`). - **`list_actions` publishes a flow action's inputs.** A `type: 'flow'` action's contract is its target flow's `isInput` variables, not `action.params`; those are now surfaced in declaration order with the `label`, `type`, `required` and select `options` of the screen field that collects each one. An action that declares its own `params[]` keeps them — the flow is read only where the action declared nothing. -**Interactive runs are unchanged.** A console launch carries the record it was launched from and that record's id — never a value for the screen's own fields — so the form renders exactly as before. That covers both shapes a launch actually supplies: a subject-record column named like one of the screen's fields, and a field named like one of the row-id keys the dispatch doors seed (`recordId`, the camelCase `Id` alias, an action's declared `recordIdParam`), none of which counts as the caller answering the screen — the two fixed names outright, and any value equal to the launched row's id whatever key carries it. One accepted cost of that: **a screen field named `recordId` or `Id` is always collected interactively**, even from a headless caller. Two screens never take the new path, because they declare nothing to satisfy and must not be answered vacuously: a message-only screen (no fields), and any screen whose author wrote `waitForInput: true`. `waitForInput: false` remains the wrong tool for the headless case — it skips the form for interactive users too. +**Interactive runs are unchanged.** A console launch carries the record it was launched from and that record's id — never a value for the screen's own fields — so the form renders exactly as before. That covers both shapes a launch actually supplies: a subject-record column named like one of the screen's fields, and a field named like one of the row-id keys the dispatch doors seed (`recordId`, the camelCase `Id` alias, an action's declared `recordIdParam`), none of which counts as the caller answering the screen. + +**Accepted cost, precisely:** a field is never treated as caller-supplied when it is named `recordId` or `Id`, or when its value equals what the bag carries under `recordId`, `Id`, or `record.id` (normally the launched row's id); a required such field is therefore always collected interactively, an optional one simply does not count as answering the screen. Two screens never take the new path, because they declare nothing to satisfy and must not be answered vacuously: a message-only screen (no fields), and any screen whose author wrote `waitForInput: true`. `waitForInput: false` remains the wrong tool for the headless case — it skips the form for interactive users too. ⚠️ One known gap, on the trigger-record leg only: a run continued from the **durable** suspended-run store judges against a JSON copy of its context, so a later wizard screen whose field collides with a **non-scalar** column (an array or object) of the trigger record can read as caller-supplied and be skipped. Scalar columns are unaffected, as is any run that has not been through a pause. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index c93926ad40..38db4ff5ec 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -753,11 +753,11 @@ The screen is treated as answered only when **all** of these hold: row id a launch carries does **not** count as naming a field: both dispatch doors seed it into the params bag under `recordId` and the camelCase `Id` alias (and under an action's declared `recordIdParam`), and none - of those is the caller supplying a screen value. The two fixed names are - refused outright, and any value equal to the launched row's id is refused - whatever key carries it — so **a screen field named `recordId` or `Id` - is always collected interactively**, even from a headless caller. Name a field - you want a headless caller to fill something else; + of those is the caller supplying a screen value. + + **Accepted cost, precisely:** a field is never treated as caller-supplied when it is named `recordId` or `Id`, or when its value equals what the bag carries under `recordId`, `Id`, or `record.id` (normally the launched row's id); a required such field is therefore always collected interactively, an optional one simply does not count as answering the screen. + + Name a field you want a headless caller to fill something else; - every `required` field has a value (an empty or blank string is no value), supplied by that caller. A value that came from the trigger record, from an earlier node or from a declared `defaultValue` does not answer a `required`