|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * A screen the CALLER already answered no longer parks the run (#15705). |
| 5 | + * |
| 6 | + * The reported dead end: an `ai.exposed` action whose target is a screen flow |
| 7 | + * can be STARTED over MCP and never finished. `run_action` seeds the flow's |
| 8 | + * `isInput` variables from the caller's `params` — `seedFlowActionParams` does |
| 9 | + * that correctly — and the screen node suspended anyway, because the only |
| 10 | + * inputs to `shouldPause` were "does the node declare fields" and the author's |
| 11 | + * `waitForInput` flag. The MCP tool set has no resume verb, so the run parked |
| 12 | + * on the screen with nothing able to continue it: `ai.exposed` meant "the agent |
| 13 | + * can invoke this", not "the agent can complete this". |
| 14 | + * |
| 15 | + * ## What this file pins, and why the controls outnumber the fix |
| 16 | + * |
| 17 | + * The fix is one line of predicate; the risk is entirely in the OTHER runs that |
| 18 | + * reach it. A screen node is also entered by interactive console runs, by |
| 19 | + * record-change and scheduled triggers, and by region bodies — and the failure |
| 20 | + * mode of getting this wrong is silent: a screen that stops rendering for a |
| 21 | + * human. So every narrowing in `judgeHeadlessScreen` has a control here, and |
| 22 | + * the interactive-still-pauses control is the load-bearing one. |
| 23 | + * |
| 24 | + * The sharpest of them is `seeded params`: the bag the engine receives from a |
| 25 | + * flow ACTION is not the caller's bag. `seedFlowActionParams` returns |
| 26 | + * `{ ...record, recordId, <objectName>Id, ...params }`, so every column of the |
| 27 | + * subject row arrives in `context.params` whether the caller named it or not. |
| 28 | + * Reading "the key is in params" as "the caller supplied it" would let an |
| 29 | + * interactive console run — which supplies nothing — skip a screen whose field |
| 30 | + * shares a name with a column of the record it was launched from. Those runs |
| 31 | + * are reproduced here through the real seeding shape, not a hand-made bag. |
| 32 | + * |
| 33 | + * ⛔ This does NOT claim screen flows are completable over MCP in general: a |
| 34 | + * call that omits the inputs still parks (pinned below), and nothing on that |
| 35 | + * surface can resume it. That half is a resume verb and is not this change. |
| 36 | + */ |
| 37 | + |
| 38 | +import { describe, it, expect, beforeEach } from 'vitest'; |
| 39 | +import { AutomationEngine } from '../engine.js'; |
| 40 | +import { installBuiltinNodes } from './index.js'; |
| 41 | +import type { AutomationContext } from '@objectstack/spec/contracts'; |
| 42 | + |
| 43 | +function silentLogger() { |
| 44 | + return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any; |
| 45 | +} |
| 46 | +function ctx() { |
| 47 | + return { logger: silentLogger(), getService() { return undefined; } } as any; |
| 48 | +} |
| 49 | + |
| 50 | +/** |
| 51 | + * The card's specimen, reduced to the platform facts: a `screen` flow whose |
| 52 | + * first node after `start` collects the same names the flow declares as |
| 53 | + * `isInput` variables. `subject` and `dueDate` are required, `notes` is not. |
| 54 | + * All three are `isOutput` too, so a completed run reports what actually bound |
| 55 | + * — "it continued" and "it continued with the caller's values" are different |
| 56 | + * claims and the second is the one worth making. |
| 57 | + */ |
| 58 | +function followupFlow(overrides: Record<string, unknown> = {}) { |
| 59 | + return { |
| 60 | + name: 'schedule_followup', |
| 61 | + label: 'Schedule Follow-up', |
| 62 | + type: 'screen', |
| 63 | + status: 'active', |
| 64 | + version: 1, |
| 65 | + variables: [ |
| 66 | + { name: 'subject', type: 'text', isInput: true, isOutput: true }, |
| 67 | + { name: 'dueDate', type: 'text', isInput: true, isOutput: true }, |
| 68 | + { name: 'notes', type: 'text', isInput: true, isOutput: true }, |
| 69 | + ], |
| 70 | + nodes: [ |
| 71 | + { id: 'start', type: 'start', label: 'Start' }, |
| 72 | + { |
| 73 | + id: 'screen_1', type: 'screen', label: 'Schedule Follow-up', |
| 74 | + config: { |
| 75 | + fields: [ |
| 76 | + { name: 'subject', label: 'Subject', type: 'text', required: true }, |
| 77 | + { name: 'dueDate', label: 'Due date', type: 'date', required: true }, |
| 78 | + { name: 'notes', label: 'Notes', type: 'text' }, |
| 79 | + ], |
| 80 | + ...overrides, |
| 81 | + }, |
| 82 | + }, |
| 83 | + { id: 'end', type: 'end', label: 'End' }, |
| 84 | + ], |
| 85 | + edges: [ |
| 86 | + { id: 'e1', source: 'start', target: 'screen_1', type: 'default' }, |
| 87 | + { id: 'e2', source: 'screen_1', target: 'end', type: 'default' }, |
| 88 | + ], |
| 89 | + }; |
| 90 | +} |
| 91 | + |
| 92 | +/** |
| 93 | + * The params bag a flow ACTION actually reaches the engine with — the subject |
| 94 | + * row first, the caller's explicit params last, exactly as |
| 95 | + * `seedFlowActionParams` (`@objectstack/runtime`) composes it. Reproduced here |
| 96 | + * rather than imported so this package's pins do not depend on the other |
| 97 | + * package's build; the shape is asserted against the real producer's |
| 98 | + * documented contract in its own suite. |
| 99 | + */ |
| 100 | +function actionContext( |
| 101 | + record: Record<string, unknown>, |
| 102 | + params: Record<string, unknown>, |
| 103 | +): AutomationContext { |
| 104 | + return { |
| 105 | + record, |
| 106 | + object: 'crm_lead', |
| 107 | + params: { ...record, recordId: record.id, crmLeadId: record.id, ...params }, |
| 108 | + } as AutomationContext; |
| 109 | +} |
| 110 | + |
| 111 | +const LEAD = { id: 'lead_1', name: 'Acme', company: 'Acme Inc' }; |
| 112 | + |
| 113 | +describe('screen headless satisfaction (#15705)', () => { |
| 114 | + let engine: AutomationEngine; |
| 115 | + |
| 116 | + beforeEach(() => { |
| 117 | + engine = new AutomationEngine(silentLogger()); |
| 118 | + installBuiltinNodes(engine, ctx()); |
| 119 | + }); |
| 120 | + |
| 121 | + function register(overrides: Record<string, unknown> = {}, flow = followupFlow(overrides)) { |
| 122 | + engine.registerFlow('schedule_followup', flow as any); |
| 123 | + } |
| 124 | + |
| 125 | + // ── The fix ─────────────────────────────────────────────────────────── |
| 126 | + |
| 127 | + it('continues past the screen when the caller supplied every required field', async () => { |
| 128 | + register(); |
| 129 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { |
| 130 | + subject: 'Call Acme back', dueDate: '2026-09-09', notes: 'left voicemail', |
| 131 | + })); |
| 132 | + expect(res.status).not.toBe('paused'); |
| 133 | + expect(res.screen).toBeUndefined(); |
| 134 | + expect(res.success).toBe(true); |
| 135 | + // The values the caller sent are what the run carried — not merely that |
| 136 | + // it did not stop. |
| 137 | + expect(res.output).toMatchObject({ |
| 138 | + subject: 'Call Acme back', dueDate: '2026-09-09', notes: 'left voicemail', |
| 139 | + }); |
| 140 | + }); |
| 141 | + |
| 142 | + it('continues when the caller supplied the REQUIRED fields and left an optional one out', async () => { |
| 143 | + register(); |
| 144 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { |
| 145 | + subject: 'Call Acme back', dueDate: '2026-09-09', |
| 146 | + })); |
| 147 | + expect(res.status).not.toBe('paused'); |
| 148 | + expect(res.success).toBe(true); |
| 149 | + expect(res.output).toMatchObject({ subject: 'Call Acme back', notes: undefined }); |
| 150 | + }); |
| 151 | + |
| 152 | + // ── The control that matters: interactive runs are untouched ────────── |
| 153 | + |
| 154 | + it('CONTROL — an interactive run (no params) still pauses and still renders the form', async () => { |
| 155 | + register(); |
| 156 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, {})); |
| 157 | + expect(res.status).toBe('paused'); |
| 158 | + expect(res.screen?.nodeId).toBe('screen_1'); |
| 159 | + expect(res.screen?.fields.map((f) => f.name)).toEqual(['subject', 'dueDate', 'notes']); |
| 160 | + expect(res.screen?.fields.find((f) => f.name === 'subject')?.required).toBe(true); |
| 161 | + }); |
| 162 | + |
| 163 | + it('CONTROL — a run with NO context at all still pauses (trigger / schedule shape)', async () => { |
| 164 | + register(); |
| 165 | + const res = await engine.execute('schedule_followup', {} as AutomationContext); |
| 166 | + expect(res.status).toBe('paused'); |
| 167 | + expect(res.screen?.nodeId).toBe('screen_1'); |
| 168 | + }); |
| 169 | + |
| 170 | + /** |
| 171 | + * The provenance leg, stated as the regression it prevents. Here the SUBJECT |
| 172 | + * ROW carries columns named exactly like the screen's required fields, so |
| 173 | + * the dispatcher's `{ ...record }` seed puts both names in `context.params` |
| 174 | + * for a run that supplied nothing. Treating "in params" as "caller supplied" |
| 175 | + * would skip this screen for a human who pressed a button. |
| 176 | + */ |
| 177 | + it('CONTROL — record columns that collide with screen field names do NOT satisfy the screen', async () => { |
| 178 | + register(); |
| 179 | + const collidingLead = { ...LEAD, subject: 'row value', dueDate: '2026-01-01' }; |
| 180 | + const res = await engine.execute('schedule_followup', actionContext(collidingLead, {})); |
| 181 | + expect(res.status).toBe('paused'); |
| 182 | + expect(res.screen?.nodeId).toBe('screen_1'); |
| 183 | + }); |
| 184 | + |
| 185 | + it('a caller that OVERRIDES a colliding column is still caller-supplied and continues', async () => { |
| 186 | + register(); |
| 187 | + const collidingLead = { ...LEAD, subject: 'row value', dueDate: '2026-01-01' }; |
| 188 | + const res = await engine.execute('schedule_followup', actionContext(collidingLead, { |
| 189 | + subject: 'Call Acme back', dueDate: '2026-09-09', |
| 190 | + })); |
| 191 | + expect(res.status).not.toBe('paused'); |
| 192 | + expect(res.output).toMatchObject({ subject: 'Call Acme back', dueDate: '2026-09-09' }); |
| 193 | + }); |
| 194 | + |
| 195 | + it('CONTROL — a partially supplied screen still pauses (one required field missing)', async () => { |
| 196 | + register(); |
| 197 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { |
| 198 | + subject: 'Call Acme back', |
| 199 | + })); |
| 200 | + expect(res.status).toBe('paused'); |
| 201 | + expect(res.screen?.nodeId).toBe('screen_1'); |
| 202 | + }); |
| 203 | + |
| 204 | + it('CONTROL — an empty string does not answer a required field', async () => { |
| 205 | + register(); |
| 206 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { |
| 207 | + subject: 'Call Acme back', dueDate: ' ', |
| 208 | + })); |
| 209 | + expect(res.status).toBe('paused'); |
| 210 | + }); |
| 211 | + |
| 212 | + // ── Vacuity guards: a screen with nothing to satisfy must not be skipped ── |
| 213 | + |
| 214 | + it('CONTROL — an explicit `waitForInput: true` still pauses even when fully supplied', async () => { |
| 215 | + register({ waitForInput: true }); |
| 216 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { |
| 217 | + subject: 'Call Acme back', dueDate: '2026-09-09', |
| 218 | + })); |
| 219 | + expect(res.status).toBe('paused'); |
| 220 | + expect(res.screen?.nodeId).toBe('screen_1'); |
| 221 | + }); |
| 222 | + |
| 223 | + it('CONTROL — a message-only screen (no fields) still pauses; a bag cannot vacuously answer it', async () => { |
| 224 | + const flow: any = followupFlow(); |
| 225 | + flow.nodes[1].config = { title: 'Confirm', waitForInput: true }; |
| 226 | + register({}, flow); |
| 227 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { |
| 228 | + subject: 'Call Acme back', dueDate: '2026-09-09', |
| 229 | + })); |
| 230 | + expect(res.status).toBe('paused'); |
| 231 | + expect(res.screen?.nodeId).toBe('screen_1'); |
| 232 | + }); |
| 233 | + |
| 234 | + it('CONTROL — an all-optional screen still pauses when the caller named none of its fields', async () => { |
| 235 | + const flow: any = followupFlow(); |
| 236 | + flow.nodes[1].config.fields = [{ name: 'notes', label: 'Notes', type: 'text' }]; |
| 237 | + register({}, flow); |
| 238 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, {})); |
| 239 | + expect(res.status).toBe('paused'); |
| 240 | + }); |
| 241 | + |
| 242 | + it('an all-optional screen the caller DID name is answered and continues', async () => { |
| 243 | + const flow: any = followupFlow(); |
| 244 | + flow.nodes[1].config.fields = [{ name: 'notes', label: 'Notes', type: 'text' }]; |
| 245 | + register({}, flow); |
| 246 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { notes: 'left voicemail' })); |
| 247 | + expect(res.status).not.toBe('paused'); |
| 248 | + expect(res.output).toMatchObject({ notes: 'left voicemail' }); |
| 249 | + }); |
| 250 | + |
| 251 | + /** |
| 252 | + * `visibleWhen` is enforced HERE and deliberately not on the resume door. |
| 253 | + * The server has no client and no collected values, so it cannot evaluate |
| 254 | + * the predicate — but refusing costs only a pause, which is what this |
| 255 | + * screen does today anyway. The resume door makes the opposite call for the |
| 256 | + * opposite reason: there, demanding a hidden field dead-ends a run at |
| 257 | + * Submit (#3528). |
| 258 | + */ |
| 259 | + it('CONTROL — a conditional required field the caller did not name keeps the screen interactive', async () => { |
| 260 | + const flow: any = followupFlow(); |
| 261 | + flow.nodes[1].config.fields = [ |
| 262 | + { name: 'subject', label: 'Subject', type: 'text', required: true }, |
| 263 | + { name: 'notes', label: 'Reason', type: 'text', required: true, visibleWhen: "subject == 'escalate'" }, |
| 264 | + ]; |
| 265 | + register({}, flow); |
| 266 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, { subject: 'Call Acme back' })); |
| 267 | + expect(res.status).toBe('paused'); |
| 268 | + }); |
| 269 | + |
| 270 | + it('CONTROL — `waitForInput: false` is still a pass-through, unchanged', async () => { |
| 271 | + register({ waitForInput: false }); |
| 272 | + const res = await engine.execute('schedule_followup', actionContext(LEAD, {})); |
| 273 | + expect(res.status).not.toBe('paused'); |
| 274 | + expect(res.success).toBe(true); |
| 275 | + }); |
| 276 | +}); |
0 commit comments