Skip to content

Commit f91c686

Browse files
committed
wip(#15705): headless screen satisfaction + flow input params
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
1 parent ef60224 commit f91c686

4 files changed

Lines changed: 261 additions & 7 deletions

File tree

packages/runtime/src/action-execution.ts

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ export function actionLooksDestructive(_deps: ActionExecutionDeps, action: any):
981981
return Boolean(action?.mode === 'delete' || action?.variant === 'danger');
982982
}
983983

984-
export function summarizeAction(deps: ActionExecutionDeps, action: any, obj: any, objectName: string): any {
984+
export function summarizeAction(deps: ActionExecutionDeps, action: any, obj: any, objectName: string, flow?: any): any {
985985
// [#15079] `operation` before `type`, on the LISTING face. A declarative
986986
// update always requires a current record — that is contract point 7, and
987987
// 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
999999
const description =
10001000
(typeof action?.ai?.description === 'string' ? action.ai.description : undefined) ??
10011001
(typeof action?.label === 'string' ? action.label : undefined);
1002-
const params = summarizeActionParams(deps, action, obj);
1002+
const params = summarizeActionParams(deps, action, obj, flow);
10031003
return {
10041004
name: action.name,
10051005
objectName,
@@ -1029,7 +1029,7 @@ export function jsonTypeOf(_deps: ActionExecutionDeps, t: string | undefined): '
10291029
}
10301030
}
10311031

1032-
export function summarizeActionParams(deps: ActionExecutionDeps, action: any, obj: any): any[] {
1032+
export function summarizeActionParams(deps: ActionExecutionDeps, action: any, obj: any, flow?: any): any[] {
10331033
const fields: Record<string, any> = obj?.fields ?? {};
10341034
const out: any[] = [];
10351035
for (const p of (Array.isArray(action?.params) ? action.params : [])) {
@@ -1055,9 +1055,89 @@ export function summarizeActionParams(deps: ActionExecutionDeps, action: any, ob
10551055
...(enumVals.length > 0 ? { enum: enumVals } : {}),
10561056
});
10571057
}
1058+
// [#15705] A FLOW action's input contract is its flow's `isInput`
1059+
// variables, not `action.params` — a flow-typed action almost never
1060+
// declares `params`, so this listing answered with no `params` key at all
1061+
// while the MCP `list_actions` tool description promised "its input
1062+
// parameters". An agent could see the action, could invoke it, and had no
1063+
// way to learn a single input name.
1064+
//
1065+
// Second, never first: a declaration the AUTHOR wrote on the action wins
1066+
// outright, so this can only fill a silence. `flow` is optional and the
1067+
// caller resolves it (`domains/mcp.ts` asks the automation service's
1068+
// `getFlow`), which keeps this function pure and leaves every existing
1069+
// 3-argument call site — and every non-flow action — byte-identical.
1070+
if (out.length === 0) out.push(...summarizeFlowInputParams(deps, flow));
1071+
return out;
1072+
}
1073+
1074+
/**
1075+
* A screen flow's input contract, projected onto the same param shape
1076+
* {@link summarizeActionParams} emits for a declared param (#15705).
1077+
*
1078+
* The flow's `isInput` variables ARE the contract — they are what
1079+
* `seedDeclaredVariables` binds from the caller's `params`, so their names are
1080+
* exactly the keys an invoker must send. The variable declaration carries only
1081+
* `name` / `type` / `defaultValue`, so everything an agent needs beyond the
1082+
* name (`label`, `required`, select `options`) is read off the screen node
1083+
* that collects the variable — the same field spec a paused run surfaces.
1084+
*
1085+
* `required` comes from the screen field alone: a flow variable has no
1086+
* `required` key, and inferring one from "declares no `defaultValue`" would
1087+
* invent a contract the author never wrote. A variable no screen collects is
1088+
* still listed — it is a real input, and omitting it would hide the very names
1089+
* this exists to publish — just without the screen-only enrichments.
1090+
*/
1091+
export function summarizeFlowInputParams(deps: ActionExecutionDeps, flow: any): any[] {
1092+
const variables: any[] = Array.isArray(flow?.variables) ? flow.variables : [];
1093+
if (variables.length === 0) return [];
1094+
const screenFields = collectScreenFieldSpecs(flow);
1095+
const out: any[] = [];
1096+
for (const v of variables) {
1097+
const name: unknown = v?.name;
1098+
if (v?.isInput !== true || typeof name !== 'string' || !name) continue;
1099+
const field = screenFields.get(name);
1100+
const type = jsonTypeOf(deps, field?.type ?? v?.type);
1101+
const description = typeof field?.label === 'string' && field.label ? field.label : undefined;
1102+
const enumVals = Array.isArray(field?.options)
1103+
? field.options
1104+
.map((o: any) => (typeof o === 'string' ? o : o?.value))
1105+
.filter((x: any): x is string => typeof x === 'string')
1106+
: [];
1107+
out.push({
1108+
name,
1109+
type,
1110+
required: field?.required === true,
1111+
...(description ? { description } : {}),
1112+
...(enumVals.length > 0 ? { enum: enumVals } : {}),
1113+
});
1114+
}
10581115
return out;
10591116
}
10601117

1118+
/**
1119+
* Every screen field a flow declares, by field name, first declaration
1120+
* winning. Walks ALL `screen` nodes rather than just the first: a multi-step
1121+
* wizard collects its inputs across several screens, and a contract that
1122+
* stopped at screen one would publish a subset while looking complete.
1123+
*
1124+
* Object-form screens contribute nothing by construction — their `fields` is
1125+
* empty because the client renders the object's own form — so they are simply
1126+
* skipped rather than special-cased.
1127+
*/
1128+
function collectScreenFieldSpecs(flow: any): Map<string, any> {
1129+
const byName = new Map<string, any>();
1130+
for (const node of Array.isArray(flow?.nodes) ? flow.nodes : []) {
1131+
if (node?.type !== 'screen') continue;
1132+
for (const field of Array.isArray(node?.config?.fields) ? node.config.fields : []) {
1133+
const name: unknown = field?.name;
1134+
if (typeof name !== 'string' || !name || byName.has(name)) continue;
1135+
byName.set(name, field);
1136+
}
1137+
}
1138+
return byName;
1139+
}
1140+
10611141
/**
10621142
* Resolve an action's declared `params[]` to their effective value-shape
10631143
* inputs (ADR-0104 D2). A field-backed param inherits type/multiple/

packages/runtime/src/domains/mcp.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,14 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon
627627
// identity forwarded. No `@objectstack/service-ai`.
628628
listActions: async () => {
629629
const meta: any = await getMeta();
630-
const hasAutomation = Boolean(await actionExec.resolveAutomationService(deps, context, envId));
630+
// [#15705] The service instance, not just its presence: a
631+
// flow-typed action's input contract lives in the FLOW, and
632+
// `getFlow` is the declared door onto it (`IAutomationService`,
633+
// the same probe `dispatchFlowAction` uses before it dispatches).
634+
// Without it `list_actions` answered with no `params` for every
635+
// flow action while promising "its input parameters".
636+
const automation: any = await actionExec.resolveAutomationService(deps, context, envId);
637+
const hasAutomation = Boolean(automation);
631638
const out: any[] = [];
632639
for (const { action, objectName, obj } of await actionExec.collectActionDeclarations(deps, meta)) {
633640
if (!objectName || isSystemObjectName(objectName)) continue; // fail-closed on sys_*
@@ -639,7 +646,19 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon
639646
if (actionExec.actionAiExposureError(deps, action)) continue;
640647
// Hide actions the caller is not permitted to run.
641648
if (actionExec.actionPermissionError(deps, action, ec)) continue;
642-
out.push(actionExec.summarizeAction(deps, action, obj, objectName));
649+
// Resolved per flow action, and only for one: a service that
650+
// omits the optional `getFlow` — or a flow the registry does
651+
// not hold — simply summarizes as before, since the fallback
652+
// is "no `params` key", exactly today's answer.
653+
let flow: any;
654+
if (action?.type === 'flow' && typeof action?.target === 'string' && typeof automation?.getFlow === 'function') {
655+
try {
656+
flow = await automation.getFlow(action.target);
657+
} catch {
658+
flow = undefined; // a registry that cannot answer is not a listing failure
659+
}
660+
}
661+
out.push(actionExec.summarizeAction(deps, action, obj, objectName, flow ?? undefined));
643662
}
644663
return out;
645664
},

packages/services/service-automation/src/builtin/screen-nodes.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { ScreenConfigParsed, ScriptConfigParsed } from '@objectstack/spec/a
66
import type { AutomationEngine } from '../engine.js';
77
import { interpolate } from './template.js';
88
import { parseNodeConfig } from './parse-config.js';
9+
import { judgeHeadlessScreen } from '../screen-input-contract.js';
910

1011
/**
1112
* Screen / Script built-in nodes — 'screen' and 'script' executors.
@@ -18,7 +19,13 @@ import { parseNodeConfig } from './parse-config.js';
1819
* to render; the run continues via `resume()` with the collected values (set
1920
* as bare flow variables). A field-less screen — or one with
2021
* `waitForInput === false` — stays a server pass-through (input vars, if any,
21-
* are already injected from `context.params`).
22+
* are already injected from `context.params`). Since #15705 a screen the
23+
* RUN'S CALLER already answered is a fourth pass-through: when the caller
24+
* supplied this screen's own fields and every `required` one is bound, there
25+
* is nothing left to collect, so the run continues rather than parking on a
26+
* form a headless invoker cannot submit. `judgeHeadlessScreen` owns that
27+
* verdict and refuses on every uncertainty, so an interactive run is
28+
* untouched.
2229
* - 'script' nodes call a registered function (#1870): `config.function` names
2330
* it, `engine.resolveFunction()` resolves it, and the host bridges that to
2431
* `bundle.functions` / `defineStack({ functions })`. A name that resolves to
@@ -162,7 +169,34 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext
162169
const hasFields = rawFields.length > 0;
163170
// Suspend to collect input when the screen declares fields, or opts in
164171
// explicitly. `waitForInput === false` forces a server pass-through.
165-
const shouldPause = cfg.waitForInput === true || (hasFields && cfg.waitForInput !== false);
172+
const wantsPause = cfg.waitForInput === true || (hasFields && cfg.waitForInput !== false);
173+
// #15705 — a screen whose inputs the CALLER already supplied is
174+
// answered, so the run continues instead of parking on a form nobody
175+
// will ever submit. A headless invoker (`run_action` over MCP) seeds
176+
// the flow's `isInput` variables and then has no resume verb; before
177+
// this, every screen-typed flow action was a dead end for it.
178+
//
179+
// Three deliberate narrowings, each keeping an existing behaviour whole:
180+
//
181+
// - `waitForInput === true` is NOT overridden. That flag is the
182+
// author's explicit "show this", and its documented job is the
183+
// field-less message / confirmation screen — a screen that collects
184+
// nothing and would therefore be VACUOUSLY satisfiable. Honouring it
185+
// keeps a confirmation step from being skipped by a bag it never
186+
// asked for.
187+
// - `hasFields` is required, for the same vacuity reason stated from
188+
// the other side: no declared fields means no contract to satisfy.
189+
// - the verdict itself refuses on every uncertainty
190+
// (`judgeHeadlessScreen`), so an interactive run — which supplies
191+
// none of the screen's own fields — takes the untouched path.
192+
//
193+
// ⛔ This does NOT make every screen flow completable over MCP: a call
194+
// that omits the inputs still parks, and nothing on that surface can
195+
// continue it. That half is a resume verb, and it is not this change.
196+
const headless = wantsPause && hasFields && cfg.waitForInput !== true
197+
? judgeHeadlessScreen(rawFields, variables, context)
198+
: undefined;
199+
const shouldPause = wantsPause && headless?.satisfied !== true;
166200
if (!shouldPause) {
167201
return { success: true };
168202
}

packages/services/service-automation/src/screen-input-contract.ts

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,124 @@ export function validateScreenInputs(
142142
export function declaredScreenFieldNames(fields: readonly ScreenFieldSpec[]): string[] {
143143
return fields.map((f) => f?.name).filter((n): n is string => typeof n === 'string' && n.length > 0);
144144
}
145+
146+
/**
147+
* A screen field, reduced to the three keys a satisfaction verdict turns on.
148+
* Structurally a {@link ScreenFieldSpec} subset, so the node executor can hand
149+
* its parsed `config.fields` straight in.
150+
*/
151+
export interface ScreenFieldContract {
152+
name: string;
153+
required?: boolean;
154+
visibleWhen?: string;
155+
}
156+
157+
/** Why a screen was (or was not) satisfied without showing it — see {@link judgeHeadlessScreen}. */
158+
export interface HeadlessScreenVerdict {
159+
/** `true` ⇒ the run may continue past this screen without suspending. */
160+
satisfied: boolean;
161+
/** Declared field names whose value this run's CALLER supplied (provenance-checked). */
162+
supplied: string[];
163+
/** Required fields with no usable bound value — the reason a candidate was refused. */
164+
missing: string[];
165+
}
166+
167+
const NOTHING_SUPPLIED: HeadlessScreenVerdict = { satisfied: false, supplied: [], missing: [] };
168+
169+
/**
170+
* Whether a screen field's value in `context.params` came from the run's
171+
* CALLER rather than from the subject record the dispatcher seeded.
172+
*
173+
* This distinction is the whole safety story of {@link judgeHeadlessScreen},
174+
* because the params bag a flow action reaches the engine with is NOT the
175+
* caller's bag: `seedFlowActionParams` (`@objectstack/runtime`) returns
176+
* `{ ...record, recordId, <objectName>Id, ...params }`, so every column of the
177+
* subject row is in there whether the caller named it or not. Reading "the key
178+
* is in `params`" as "the caller supplied it" would let an INTERACTIVE console
179+
* run — which supplies nothing — skip a screen whose field happens to share a
180+
* name with a column of the record it was launched from.
181+
*
182+
* Two legs, either of which proves caller provenance:
183+
*
184+
* - the record has no such key at all ⇒ the record leg cannot be the source;
185+
* - the record HAS the key but `params` holds a different value ⇒ the
186+
* caller's bag overwrote it. `{ ...record }` copies the record's own value
187+
* by reference/primitive, so a run that supplied nothing is `Object.is`-equal
188+
* here, always. Equality is therefore "indistinguishable", not "caller-set".
189+
*
190+
* The ambiguous case (same key, same value) resolves to NOT caller-supplied,
191+
* which costs a headless run a pause it might have been allowed to skip and
192+
* costs an interactive run nothing. That asymmetry is deliberate: every
193+
* uncertainty in this module must land on today's behaviour.
194+
*/
195+
function callerSupplied(
196+
name: string,
197+
context: { params?: Record<string, unknown>; record?: Record<string, unknown> } | undefined,
198+
): boolean {
199+
const params = context?.params;
200+
if (!params || params[name] === undefined) return false;
201+
const record = context?.record;
202+
if (!record || !Object.prototype.hasOwnProperty.call(record, name)) return true;
203+
return !Object.is(params[name], record[name]);
204+
}
205+
206+
/**
207+
* Can this screen be treated as already answered, and the run continued,
208+
* without suspending to show it? (#15705)
209+
*
210+
* The defect this answers: an `ai.exposed` action whose target is a screen
211+
* flow could be STARTED over MCP and never finished. `run_action` seeds the
212+
* flow's `isInput` variables from the caller's `params` — correctly — and the
213+
* screen node then suspended anyway, because the only inputs to that decision
214+
* were "does the node declare fields" and the author's `waitForInput` flag.
215+
* The MCP tool set has no resume verb, so the run parked forever.
216+
*
217+
* ⛔ NOT a general "skip screens" switch. Three conditions must ALL hold, and
218+
* the verdict is `false` the moment any of them is unproven:
219+
*
220+
* 1. **The caller supplied at least one of THIS screen's declared fields**
221+
* ({@link callerSupplied}). Without this leg a screen whose fields are all
222+
* optional would be vacuously "satisfied" and would stop rendering for
223+
* everyone — the loudest way to break the interactive path. A run that
224+
* named none of this screen's fields is not driving it, so it pauses.
225+
* 2. **Every `required` field has a usable value bound** in the live flow
226+
* variables — judged by {@link validateScreenInputs}, the same function
227+
* the resume door enforces the same contract with, so "present" cannot
228+
* drift into two meanings (an empty string is absent on both).
229+
* 3. Only caller-supplied names enter the bag, so a required field bound
230+
* from the record, from a prior node or from a declared `defaultValue`
231+
* does NOT count as answered. Optional fields are free to come from
232+
* anywhere — they constrain nothing.
233+
*
234+
* **`visibleWhen` is enforced here, the OPPOSITE of the resume door**, and the
235+
* asymmetry is the point rather than an oversight. On resume, an unevaluable
236+
* predicate must not fire `required`: the client is the authority on what the
237+
* user was shown, and demanding a hidden field dead-ends a run at Submit
238+
* (#3528). Here the server has no client and no collected values, so it cannot
239+
* evaluate the predicate either — but refusing costs nothing except a pause,
240+
* which is exactly what this screen does today. So a conditional required field
241+
* the caller did not name keeps the screen interactive.
242+
*/
243+
export function judgeHeadlessScreen(
244+
fields: readonly ScreenFieldContract[],
245+
variables: ReadonlyMap<string, unknown>,
246+
context: { params?: Record<string, unknown>; record?: Record<string, unknown> } | undefined,
247+
): HeadlessScreenVerdict {
248+
const declared = fields.filter((f) => typeof f?.name === 'string' && f.name.length > 0);
249+
if (declared.length === 0) return NOTHING_SUPPLIED;
250+
251+
const supplied: string[] = [];
252+
const bag: Record<string, unknown> = {};
253+
for (const field of declared) {
254+
if (!callerSupplied(field.name, context)) continue;
255+
supplied.push(field.name);
256+
bag[field.name] = variables.get(field.name);
257+
}
258+
// Condition 1 — nobody drove this screen, so it stays interactive.
259+
if (supplied.length === 0) return NOTHING_SUPPLIED;
260+
261+
// Condition 2/3 — `unknown_field` cannot fire: every bag key is a declared
262+
// field by construction, so every issue returned here is a missing `required`.
263+
const issues = validateScreenInputs(declared, bag, () => true);
264+
return { satisfied: issues.length === 0, supplied, missing: issues.map((i) => i.field) };
265+
}

0 commit comments

Comments
 (0)