fix(service-automation): compare the screen caller-provenance record leg by value, so a durable resume cannot skip a screen

.changeset/screen-provenance-record-leg-by-value.md

@@ -0,0 +1,15 @@
+---
+"@objectstack/service-automation": patch
+---
+
+A wizard screen is no longer skipped after a durable pause because a record column happens to be an array.
+
+`judgeHeadlessScreen` decides a screen was already answered by proving the negative: a field is **not** caller-supplied when the subject record carries that key and `params` holds the same value — necessary because the params bag a flow action arrives with is `{ ...record, recordId, <object>Id, ...params }`, so every column of the launched row is in there whether the caller named it or not.
+
+That comparison was reference identity (`Object.is`), which is real in memory and does not survive persistence. A suspended run stores its context as JSON and resumes from the parsed copy — and the store is preferred over the in-process cache whenever one is wired, so no restart is needed. After that round trip an **array or object** column is equal but no longer identical: the record leg could not disprove it, the field read as caller-supplied, and a later screen with no required fields of its own was **skipped on a run that had supplied nothing**. An interactive user pressed a button and never saw a form they should have been shown; the run completed carrying the row's own value as if they had typed it. Reproduced end to end against a wired store, not inferred.
+
+The record leg now compares by value (`isDeepStrictEqual`), which survives serialisation. That predicate compares primitives with `Object.is` itself, so this is a strict widening of the "not caller-supplied" set — every pair the old check called equal it still calls equal, plus the structurally identical non-primitives. More screens render, never fewer, which is the direction this module resolves every ambiguity in.
+
+**Accepted cost, precisely.** A caller that genuinely re-sends a value structurally identical to the row's column is no longer distinguishable from the dispatcher's seed, so it now gets the screen rendered instead of skipped — a lost skip on a headless call, never a lost run, and the same trade the module's other legs already make. Scalar columns behave exactly as before, on both sides of a pause. The row-id leg keeps identity comparison deliberately: a row id is a scalar by construction, so serialisation cannot defeat it and there is nothing there to widen. Measured overhead is a deep compare per declared screen field at screen entry: ~1.5 µs added for a deliberately maximal screen that declares a field for every one of a ten-column row, which is about 38% of one `JSON.stringify` of the run context — a cost the durable store already pays on every suspend.
+
+This closes the gap the same release's screen-flow headless-satisfaction note records as known.

packages/services/service-automation/src/builtin/screen-headless-provenance-durable-resume.test.ts

@@ -0,0 +1,238 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The caller-provenance record leg survives a DURABLE resume (#15812).
+ *
+ * `judgeHeadlessScreen` (#15705) lets a screen continue when the CALLER already
+ * answered it. It cannot read that off `context.params`, because the params bag
+ * a flow action reaches the engine with is not the caller's bag —
+ * `seedFlowActionParams` spreads the whole subject row in first. So it proves
+ * the NEGATIVE instead: a key is not caller-supplied when the record carries it
+ * and `params` holds the same value.
+ *
+ * That leg was written as `Object.is`, i.e. as reference identity, and the
+ * record spread does copy the record's own value by reference — so in memory a
+ * run that supplied nothing IS identity-equal there. **Persistence destroys
+ * that.** A suspended run persists its `context` as JSON
+ * (`suspended-run-store.ts`: `context_json: JSON.stringify(...)` on save,
+ * `parseJson` on load) and `resumeInternal` continues the run with the parsed
+ * value — and `loadSuspendedRunStrict` prefers the STORE over the hot cache
+ * whenever one is wired, so it does not take a process restart. After that
+ * round trip `params.tags` and `record.tags` are equal but no longer identical:
+ * the record leg could not disprove them, the field read as caller-supplied,
+ * and a later all-optional screen was SKIPPED on a run that had supplied
+ * nothing.
+ *
+ * The direction is the one #15705 exists to prevent: an INTERACTIVE run
+ * skipping a screen it should have rendered. Hence the remedy here — compare by
+ * VALUE, which survives serialisation.
+ *
+ * ## The rig, and why it is shaped like this
+ *
+ * The card lists the conjunction that has to hold together, and every clause is
+ * load-bearing, so the rig satisfies all of them at once rather than stubbing
+ * any: the **actions door** (so the row is spread into `params`), a **wired
+ * durable store**, **a later screen in the same run** entered after the resume,
+ * a **non-primitive** colliding column, and **no other required field** on that
+ * screen to force the pause anyway. Remove any one and the run pauses for a
+ * different reason, which is what the controls below are for — they are not
+ * decoration, they are the evidence that the pause the fixed code produces
+ * comes from this leg and not from one of the others.
+ *
+ * ⚠️ Primitive columns were never affected: `Object.is('x','x')` is true across
+ * a round trip. `records the primitive column` below is the control that keeps
+ * that boundary honest — it passes before AND after the fix, so it cannot be
+ * mistaken for evidence of the fix.
+ *
+ * REVERT-PROOF: put `Object.is` back on the record leg of `callerSupplied` and
+ * `THE BUG` fails (the run completes instead of pausing) while every control
+ * here stays green.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { AutomationEngine } from '../engine.js';
+import { registerScreenNodes } from './screen-nodes.js';
+import { InMemorySuspendedRunStore } from '../suspended-run-store.js';
+import type { SuspendedRunStore } from '../engine.js';
+import type { AutomationContext } from '@objectstack/spec/contracts';
+
+function silentLogger() {
+    return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any;
+}
+
+/**
+ * Two screens, deliberately. The FIRST one pauses (a required field nobody
+ * supplied) — that is what puts the context through the store. The SECOND is
+ * the one under test: a single OPTIONAL field, so nothing but the provenance
+ * verdict can decide whether it renders.
+ */
+function twoScreenFlow(secondField: string) {
+    return {
+        name: 'lead_review',
+        label: 'Lead review',
+        type: 'screen',
+        status: 'active',
+        version: 1,
+        variables: [
+            { name: 'full_name', type: 'text', isInput: true, isOutput: true },
+            { name: secondField, type: 'text', isInput: true, isOutput: true },
+        ],
+        nodes: [
+            { id: 'start', type: 'start', label: 'Start' },
+            {
+                id: 'collect', type: 'screen', label: 'Your details',
+                config: { title: 'Your details', fields: [{ name: 'full_name', label: 'Full name', type: 'text', required: true }] },
+            },
+            {
+                id: 'review', type: 'screen', label: 'Review',
+                // All-optional and single-field: no `required` can force this
+                // pause, so "it paused" means exactly "nothing was judged
+                // caller-supplied".
+                config: { title: 'Review', fields: [{ name: secondField, label: 'Review', type: 'text' }] },
+            },
+            { id: 'end', type: 'end', label: 'End' },
+        ],
+        edges: [
+            { id: 'e1', source: 'start', target: 'collect', type: 'default' },
+            { id: 'e2', source: 'collect', target: 'review', type: 'default' },
+            { id: 'e3', source: 'review', target: 'end', type: 'default' },
+        ],
+    } as any;
+}
+
+/** A fresh engine over `store` (or none) — one per simulated process lifetime. */
+function buildEngine(secondField: string, store?: SuspendedRunStore) {
+    const e = new AutomationEngine(silentLogger(), store);
+    registerScreenNodes(e, { logger: silentLogger() } as any);
+    e.registerFlow('lead_review', twoScreenFlow(secondField));
+    return e;
+}
+
+/**
+ * The bag a flow ACTION actually reaches the engine with — the subject row
+ * first, the caller's own params last, exactly as `seedFlowActionParams`
+ * (`@objectstack/runtime`) composes it. Reproduced rather than imported so this
+ * package's pins do not depend on the other package's build, matching
+ * `screen-headless-satisfaction.test.ts`.
+ */
+function actionContext(
+    record: Record<string, unknown>,
+    params: Record<string, unknown> = {},
+): AutomationContext {
+    return {
+        record,
+        object: 'crm_lead',
+        params: { ...record, recordId: record.id, crmLeadId: record.id, ...params },
+    } as AutomationContext;
+}
+
+/** A row whose `tags` column is an ARRAY — the shape identity cannot survive. */
+function lead() {
+    return { id: 'lead_1', tags: ['a', 'b'], company: 'Acme Inc' };
+}
+
+/**
+ * Drive the whole conjunction: launch through the actions door, pause on the
+ * first screen, resume, and report what the SECOND screen did.
+ */
+async function driveThroughResume(
+    secondField: string,
+    store: SuspendedRunStore | undefined,
+    context: AutomationContext,
+) {
+    const engine = buildEngine(secondField, store);
+    const paused = await engine.execute('lead_review', context);
+    // Precondition, asserted rather than assumed: if the first screen did not
+    // park, nothing below went through the store and the case is vacuous.
+    expect(paused.status).toBe('paused');
+    expect(paused.screen?.nodeId).toBe('collect');
+    return engine.resume(paused.runId!, { variables: { full_name: 'Ada' } });
+}
+
+describe('caller-provenance survives a durable resume (#15812)', () => {
+    /**
+     * THE BUG. Every clause of the card's conjunction holds: actions door,
+     * wired store, a later screen after the resume, a non-primitive colliding
+     * column, and no other required field.
+     *
+     * The caller supplied NOTHING — `params.tags` is there only because the
+     * dispatcher spread the row in. So the review screen must render.
+     */
+    it('THE BUG — an array column does not answer a later screen after a durable resume', async () => {
+        const resumed = await driveThroughResume('tags', new InMemorySuspendedRunStore(), actionContext(lead()));
+        expect(resumed.status).toBe('paused');
+        expect(resumed.screen?.nodeId).toBe('review');
+        // Not merely "it stopped": it stopped WITHOUT having answered itself
+        // from the row.
+        expect((resumed.output as Record<string, unknown> | undefined)?.tags).toBeUndefined();
+    });
+
+    /**
+     * The same run with NO store: the engine resumes from its hot cache, where
+     * `params.tags` is still the very array `record.tags` is, so identity holds
+     * and the leg worked even before the fix. Green on both sides — its job is
+     * to localise the defect to the serialisation boundary, not to the screen
+     * logic.
+     */
+    it('CONTROL — with no store wired the same run pauses too (identity never left memory)', async () => {
+        const resumed = await driveThroughResume('tags', undefined, actionContext(lead()));
+        expect(resumed.status).toBe('paused');
+        expect(resumed.screen?.nodeId).toBe('review');
+    });
+
+    /**
+     * The card's own ⚠️: a PRIMITIVE column is unaffected, because
+     * `Object.is('Acme Inc', 'Acme Inc')` is true across a round trip. Green
+     * before and after — ⛔ never read this one as evidence of the fix.
+     */
+    it('CONTROL — a primitive colliding column was already refused across the round trip', async () => {
+        const resumed = await driveThroughResume('company', new InMemorySuspendedRunStore(), actionContext(lead()));
+        expect(resumed.status).toBe('paused');
+        expect(resumed.screen?.nodeId).toBe('review');
+    });
+
+    /**
+     * The row-id leg reads scalars, so serialisation cannot defeat it either.
+     * Pinned because the fix deliberately leaves that leg on `Object.is` — this
+     * is the case that says the decision was measured, not overlooked.
+     */
+    it('CONTROL — the row-id seed leg still refuses across a durable resume', async () => {
+        const resumed = await driveThroughResume('recordId', new InMemorySuspendedRunStore(), actionContext(lead()));
+        expect(resumed.status).toBe('paused');
+        expect(resumed.screen?.nodeId).toBe('review');
+    });
+
+    /**
+     * #15705's own purpose, preserved: a caller who genuinely drove the screen
+     * still continues past it after a durable resume. Without this the fix
+     * could "pass" by making everything pause.
+     */
+    it('a caller who genuinely supplied a DIFFERENT value still continues after the resume', async () => {
+        const resumed = await driveThroughResume(
+            'tags', new InMemorySuspendedRunStore(), actionContext(lead(), { tags: ['urgent'] }),
+        );
+        expect(resumed.status).not.toBe('paused');
+        expect(resumed.success).toBe(true);
+        expect(resumed.output).toMatchObject({ tags: ['urgent'] });
+    });
+
+    /**
+     * THE WIDENING, pinned as behaviour rather than left as prose.
+     *
+     * Value equality enlarges the not-caller-supplied set: a caller that
+     * re-sends a value structurally identical to the row's now reads as
+     * indistinguishable from the record seed and the screen renders. Under
+     * `Object.is` this run CONTINUED (two distinct arrays), so this case is a
+     * deliberate behaviour change and it changes in this module's standing
+     * direction — every ambiguity resolves to pausing, which costs a headless
+     * run a skip and costs an interactive run nothing.
+     *
+     * No store here: the widening is a property of the comparison, not of
+     * persistence.
+     */
+    it('WIDENING — a caller re-sending a value equal to the row is now indistinguishable, so it pauses', async () => {
+        const resumed = await driveThroughResume('tags', undefined, actionContext(lead(), { tags: ['a', 'b'] }));
+        expect(resumed.status).toBe('paused');
+        expect(resumed.screen?.nodeId).toBe('review');
+    });
+});

packages/services/service-automation/src/builtin/screen-headless-satisfaction.test.ts

@@ -396,7 +396,7 @@ describe('screen headless satisfaction (#15705)', () => {
      * 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
+     * pauses because every key still holds 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 () => {

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

@@ -21,6 +21,8 @@
  * caller.
  */
 
+import { isDeepStrictEqual } from 'node:util';
+
 import type { ScreenFieldSpec, ScreenSpec } from '@objectstack/spec/contracts';
 import type { FieldErrorCode } from '@objectstack/spec/api';
 
@@ -195,23 +197,42 @@ const NOTHING_SUPPLIED: HeadlessScreenVerdict = { satisfied: false, supplied: []
  *
  *  - 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. The record spread copies the record's own value
- *    by reference/primitive, so a run that supplied nothing is `Object.is`-equal
- *    here. Equality is therefore "indistinguishable", not "caller-set".
+ *    caller's bag overwrote it. The record spread copies the record's own
+ *    value, so a run that supplied nothing carries the row's value 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.
+ * **The record leg compares by VALUE, and had to (#15812).** Written as
+ * `Object.is` it asked about reference identity, which is real in memory — the
+ * record spread copies by reference — and is destroyed by persistence. A
+ * suspended run persists its `context` as JSON (`suspended-run-store.ts`:
+ * `JSON.stringify` on save, `parseJson` on load) and `resumeInternal` continues
+ * the run with the parsed value; `loadSuspendedRunStrict` prefers the store
+ * over the hot cache whenever one is wired, so this needs no process restart.
+ * After that round trip a NON-primitive column value (an array, an object) is
+ * equal but no longer identical, the record leg could not disprove it, and a
+ * later all-optional screen was SKIPPED on a run that had supplied nothing —
+ * measured end to end through a wired store, which is the failure #15705 exists
+ * to prevent. Primitive columns were never affected (`Object.is('x','x')` is
+ * true across a round trip), which is exactly why the hole was invisible to
+ * every in-memory unit test.
+ *
+ * `isDeepStrictEqual` compares primitives with `Object.is` itself, so this is a
+ * strict WIDENING of the old predicate: every pair the identity check called
+ * equal it still calls equal, plus the structurally-identical non-primitives.
+ * The widened set is "not caller-supplied", i.e. more pauses, so the change can
+ * only move runs toward this module's standing direction. The price is that a
+ * caller who genuinely re-sends a value identical to the row's is no longer
+ * distinguishable from the seed and gets the screen rendered — a lost skip, not
+ * a lost run, and the same trade every other leg here already makes.
+ *
+ * ⛔ The row-id leg above deliberately keeps `Object.is`: a row id is a scalar
+ * by construction (`params.recordId` is seeded as one, `record.id` is one), so
+ * serialisation cannot defeat it and there is nothing there to widen.
  */
 function callerSupplied(
   name: string,
@@ -251,7 +272,10 @@ function callerSupplied(
   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(value, record[name]);
+  // BY VALUE, not by identity (#15812) — see the note above. `Object.is` here
+  // read as "the caller overwrote the column" for any non-primitive value that
+  // had been through the durable store's JSON round trip.
+  return !isDeepStrictEqual(value, record[name]);
 }
 
 /**