|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * The caller-provenance record leg survives a DURABLE resume (#15812). |
| 5 | + * |
| 6 | + * `judgeHeadlessScreen` (#15705) lets a screen continue when the CALLER already |
| 7 | + * answered it. It cannot read that off `context.params`, because the params bag |
| 8 | + * a flow action reaches the engine with is not the caller's bag — |
| 9 | + * `seedFlowActionParams` spreads the whole subject row in first. So it proves |
| 10 | + * the NEGATIVE instead: a key is not caller-supplied when the record carries it |
| 11 | + * and `params` holds the same value. |
| 12 | + * |
| 13 | + * That leg was written as `Object.is`, i.e. as reference identity, and the |
| 14 | + * record spread does copy the record's own value by reference — so in memory a |
| 15 | + * run that supplied nothing IS identity-equal there. **Persistence destroys |
| 16 | + * that.** A suspended run persists its `context` as JSON |
| 17 | + * (`suspended-run-store.ts`: `context_json: JSON.stringify(...)` on save, |
| 18 | + * `parseJson` on load) and `resumeInternal` continues the run with the parsed |
| 19 | + * value — and `loadSuspendedRunStrict` prefers the STORE over the hot cache |
| 20 | + * whenever one is wired, so it does not take a process restart. After that |
| 21 | + * round trip `params.tags` and `record.tags` are equal but no longer identical: |
| 22 | + * the record leg could not disprove them, the field read as caller-supplied, |
| 23 | + * and a later all-optional screen was SKIPPED on a run that had supplied |
| 24 | + * nothing. |
| 25 | + * |
| 26 | + * The direction is the one #15705 exists to prevent: an INTERACTIVE run |
| 27 | + * skipping a screen it should have rendered. Hence the remedy here — compare by |
| 28 | + * VALUE, which survives serialisation. |
| 29 | + * |
| 30 | + * ## The rig, and why it is shaped like this |
| 31 | + * |
| 32 | + * The card lists the conjunction that has to hold together, and every clause is |
| 33 | + * load-bearing, so the rig satisfies all of them at once rather than stubbing |
| 34 | + * any: the **actions door** (so the row is spread into `params`), a **wired |
| 35 | + * durable store**, **a later screen in the same run** entered after the resume, |
| 36 | + * a **non-primitive** colliding column, and **no other required field** on that |
| 37 | + * screen to force the pause anyway. Remove any one and the run pauses for a |
| 38 | + * different reason, which is what the controls below are for — they are not |
| 39 | + * decoration, they are the evidence that the pause the fixed code produces |
| 40 | + * comes from this leg and not from one of the others. |
| 41 | + * |
| 42 | + * ⚠️ Primitive columns were never affected: `Object.is('x','x')` is true across |
| 43 | + * a round trip. `records the primitive column` below is the control that keeps |
| 44 | + * that boundary honest — it passes before AND after the fix, so it cannot be |
| 45 | + * mistaken for evidence of the fix. |
| 46 | + * |
| 47 | + * REVERT-PROOF: put `Object.is` back on the record leg of `callerSupplied` and |
| 48 | + * `THE BUG` fails (the run completes instead of pausing) while every control |
| 49 | + * here stays green. |
| 50 | + */ |
| 51 | + |
| 52 | +import { describe, it, expect } from 'vitest'; |
| 53 | +import { AutomationEngine } from '../engine.js'; |
| 54 | +import { registerScreenNodes } from './screen-nodes.js'; |
| 55 | +import { InMemorySuspendedRunStore } from '../suspended-run-store.js'; |
| 56 | +import type { SuspendedRunStore } from '../engine.js'; |
| 57 | +import type { AutomationContext } from '@objectstack/spec/contracts'; |
| 58 | + |
| 59 | +function silentLogger() { |
| 60 | + return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any; |
| 61 | +} |
| 62 | + |
| 63 | +/** |
| 64 | + * Two screens, deliberately. The FIRST one pauses (a required field nobody |
| 65 | + * supplied) — that is what puts the context through the store. The SECOND is |
| 66 | + * the one under test: a single OPTIONAL field, so nothing but the provenance |
| 67 | + * verdict can decide whether it renders. |
| 68 | + */ |
| 69 | +function twoScreenFlow(secondField: string) { |
| 70 | + return { |
| 71 | + name: 'lead_review', |
| 72 | + label: 'Lead review', |
| 73 | + type: 'screen', |
| 74 | + status: 'active', |
| 75 | + version: 1, |
| 76 | + variables: [ |
| 77 | + { name: 'full_name', type: 'text', isInput: true, isOutput: true }, |
| 78 | + { name: secondField, type: 'text', isInput: true, isOutput: true }, |
| 79 | + ], |
| 80 | + nodes: [ |
| 81 | + { id: 'start', type: 'start', label: 'Start' }, |
| 82 | + { |
| 83 | + id: 'collect', type: 'screen', label: 'Your details', |
| 84 | + config: { title: 'Your details', fields: [{ name: 'full_name', label: 'Full name', type: 'text', required: true }] }, |
| 85 | + }, |
| 86 | + { |
| 87 | + id: 'review', type: 'screen', label: 'Review', |
| 88 | + // All-optional and single-field: no `required` can force this |
| 89 | + // pause, so "it paused" means exactly "nothing was judged |
| 90 | + // caller-supplied". |
| 91 | + config: { title: 'Review', fields: [{ name: secondField, label: 'Review', type: 'text' }] }, |
| 92 | + }, |
| 93 | + { id: 'end', type: 'end', label: 'End' }, |
| 94 | + ], |
| 95 | + edges: [ |
| 96 | + { id: 'e1', source: 'start', target: 'collect', type: 'default' }, |
| 97 | + { id: 'e2', source: 'collect', target: 'review', type: 'default' }, |
| 98 | + { id: 'e3', source: 'review', target: 'end', type: 'default' }, |
| 99 | + ], |
| 100 | + } as any; |
| 101 | +} |
| 102 | + |
| 103 | +/** A fresh engine over `store` (or none) — one per simulated process lifetime. */ |
| 104 | +function buildEngine(secondField: string, store?: SuspendedRunStore) { |
| 105 | + const e = new AutomationEngine(silentLogger(), store); |
| 106 | + registerScreenNodes(e, { logger: silentLogger() } as any); |
| 107 | + e.registerFlow('lead_review', twoScreenFlow(secondField)); |
| 108 | + return e; |
| 109 | +} |
| 110 | + |
| 111 | +/** |
| 112 | + * The bag a flow ACTION actually reaches the engine with — the subject row |
| 113 | + * first, the caller's own params last, exactly as `seedFlowActionParams` |
| 114 | + * (`@objectstack/runtime`) composes it. Reproduced rather than imported so this |
| 115 | + * package's pins do not depend on the other package's build, matching |
| 116 | + * `screen-headless-satisfaction.test.ts`. |
| 117 | + */ |
| 118 | +function actionContext( |
| 119 | + record: Record<string, unknown>, |
| 120 | + params: Record<string, unknown> = {}, |
| 121 | +): AutomationContext { |
| 122 | + return { |
| 123 | + record, |
| 124 | + object: 'crm_lead', |
| 125 | + params: { ...record, recordId: record.id, crmLeadId: record.id, ...params }, |
| 126 | + } as AutomationContext; |
| 127 | +} |
| 128 | + |
| 129 | +/** A row whose `tags` column is an ARRAY — the shape identity cannot survive. */ |
| 130 | +function lead() { |
| 131 | + return { id: 'lead_1', tags: ['a', 'b'], company: 'Acme Inc' }; |
| 132 | +} |
| 133 | + |
| 134 | +/** |
| 135 | + * Drive the whole conjunction: launch through the actions door, pause on the |
| 136 | + * first screen, resume, and report what the SECOND screen did. |
| 137 | + */ |
| 138 | +async function driveThroughResume( |
| 139 | + secondField: string, |
| 140 | + store: SuspendedRunStore | undefined, |
| 141 | + context: AutomationContext, |
| 142 | +) { |
| 143 | + const engine = buildEngine(secondField, store); |
| 144 | + const paused = await engine.execute('lead_review', context); |
| 145 | + // Precondition, asserted rather than assumed: if the first screen did not |
| 146 | + // park, nothing below went through the store and the case is vacuous. |
| 147 | + expect(paused.status).toBe('paused'); |
| 148 | + expect(paused.screen?.nodeId).toBe('collect'); |
| 149 | + return engine.resume(paused.runId!, { variables: { full_name: 'Ada' } }); |
| 150 | +} |
| 151 | + |
| 152 | +describe('caller-provenance survives a durable resume (#15812)', () => { |
| 153 | + /** |
| 154 | + * THE BUG. Every clause of the card's conjunction holds: actions door, |
| 155 | + * wired store, a later screen after the resume, a non-primitive colliding |
| 156 | + * column, and no other required field. |
| 157 | + * |
| 158 | + * The caller supplied NOTHING — `params.tags` is there only because the |
| 159 | + * dispatcher spread the row in. So the review screen must render. |
| 160 | + */ |
| 161 | + it('THE BUG — an array column does not answer a later screen after a durable resume', async () => { |
| 162 | + const resumed = await driveThroughResume('tags', new InMemorySuspendedRunStore(), actionContext(lead())); |
| 163 | + expect(resumed.status).toBe('paused'); |
| 164 | + expect(resumed.screen?.nodeId).toBe('review'); |
| 165 | + // Not merely "it stopped": it stopped WITHOUT having answered itself |
| 166 | + // from the row. |
| 167 | + expect((resumed.output as Record<string, unknown> | undefined)?.tags).toBeUndefined(); |
| 168 | + }); |
| 169 | + |
| 170 | + /** |
| 171 | + * The same run with NO store: the engine resumes from its hot cache, where |
| 172 | + * `params.tags` is still the very array `record.tags` is, so identity holds |
| 173 | + * and the leg worked even before the fix. Green on both sides — its job is |
| 174 | + * to localise the defect to the serialisation boundary, not to the screen |
| 175 | + * logic. |
| 176 | + */ |
| 177 | + it('CONTROL — with no store wired the same run pauses too (identity never left memory)', async () => { |
| 178 | + const resumed = await driveThroughResume('tags', undefined, actionContext(lead())); |
| 179 | + expect(resumed.status).toBe('paused'); |
| 180 | + expect(resumed.screen?.nodeId).toBe('review'); |
| 181 | + }); |
| 182 | + |
| 183 | + /** |
| 184 | + * The card's own ⚠️: a PRIMITIVE column is unaffected, because |
| 185 | + * `Object.is('Acme Inc', 'Acme Inc')` is true across a round trip. Green |
| 186 | + * before and after — ⛔ never read this one as evidence of the fix. |
| 187 | + */ |
| 188 | + it('CONTROL — a primitive colliding column was already refused across the round trip', async () => { |
| 189 | + const resumed = await driveThroughResume('company', new InMemorySuspendedRunStore(), actionContext(lead())); |
| 190 | + expect(resumed.status).toBe('paused'); |
| 191 | + expect(resumed.screen?.nodeId).toBe('review'); |
| 192 | + }); |
| 193 | + |
| 194 | + /** |
| 195 | + * The row-id leg reads scalars, so serialisation cannot defeat it either. |
| 196 | + * Pinned because the fix deliberately leaves that leg on `Object.is` — this |
| 197 | + * is the case that says the decision was measured, not overlooked. |
| 198 | + */ |
| 199 | + it('CONTROL — the row-id seed leg still refuses across a durable resume', async () => { |
| 200 | + const resumed = await driveThroughResume('recordId', new InMemorySuspendedRunStore(), actionContext(lead())); |
| 201 | + expect(resumed.status).toBe('paused'); |
| 202 | + expect(resumed.screen?.nodeId).toBe('review'); |
| 203 | + }); |
| 204 | + |
| 205 | + /** |
| 206 | + * #15705's own purpose, preserved: a caller who genuinely drove the screen |
| 207 | + * still continues past it after a durable resume. Without this the fix |
| 208 | + * could "pass" by making everything pause. |
| 209 | + */ |
| 210 | + it('a caller who genuinely supplied a DIFFERENT value still continues after the resume', async () => { |
| 211 | + const resumed = await driveThroughResume( |
| 212 | + 'tags', new InMemorySuspendedRunStore(), actionContext(lead(), { tags: ['urgent'] }), |
| 213 | + ); |
| 214 | + expect(resumed.status).not.toBe('paused'); |
| 215 | + expect(resumed.success).toBe(true); |
| 216 | + expect(resumed.output).toMatchObject({ tags: ['urgent'] }); |
| 217 | + }); |
| 218 | + |
| 219 | + /** |
| 220 | + * THE WIDENING, pinned as behaviour rather than left as prose. |
| 221 | + * |
| 222 | + * Value equality enlarges the not-caller-supplied set: a caller that |
| 223 | + * re-sends a value structurally identical to the row's now reads as |
| 224 | + * indistinguishable from the record seed and the screen renders. Under |
| 225 | + * `Object.is` this run CONTINUED (two distinct arrays), so this case is a |
| 226 | + * deliberate behaviour change and it changes in this module's standing |
| 227 | + * direction — every ambiguity resolves to pausing, which costs a headless |
| 228 | + * run a skip and costs an interactive run nothing. |
| 229 | + * |
| 230 | + * No store here: the widening is a property of the comparison, not of |
| 231 | + * persistence. |
| 232 | + */ |
| 233 | + it('WIDENING — a caller re-sending a value equal to the row is now indistinguishable, so it pauses', async () => { |
| 234 | + const resumed = await driveThroughResume('tags', undefined, actionContext(lead(), { tags: ['a', 'b'] })); |
| 235 | + expect(resumed.status).toBe('paused'); |
| 236 | + expect(resumed.screen?.nodeId).toBe('review'); |
| 237 | + }); |
| 238 | +}); |
0 commit comments