Skip to content

Commit a36b526

Browse files
claude[bot]claude
andauthored
docs(automation): three declaration-truth residues on the run-history row — a one-way discriminator, the row's step log, and the write that names its reader (#17549)
* docs(automation): correct three declaration-truth residues on the run-history row `variables_json`'s description stated presence and "this run had a pause its resume consumed" as EQUIVALENT. Only the forward direction is true: the terminal write is an upsert that always writes the four snapshot columns, NULL included, so a run that stranded, was restored and then finished has its row rewritten with NULLs — leaving it indistinguishable, in exactly those columns, from a run that never paused. The text is what was wrong; the clearing is the ruled design, and nothing reads the reverse direction. `deserializeConsumedSuspension` described the rebuilt snapshot's steps as the step log AS OF THE PAUSE. That is true of the engine's process-local journal copy only (it slices `run.steps` back to the step count at the pause); the row's `steps_json` is the terminal row's log of the WHOLE run, and both bounds on it — history compaction and the byte cap — deliberately keep the failure and trim the head. `recordTerminal` now names the verb that reads what it writes, and the three properties of this write that are that verb's inputs. Its summary line also said "completed / failed" where the terminal vocabulary has had four members since #15223. Pins the two falsifying shapes in `suspended-run-store.test.ts`; the cross-reference has no executable surface and is not pinned. Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW Co-authored-by: Claude <noreply@anthropic.com> * chore(changeset): patch for the run-history declaration-truth corrections The `variables_json` description reaches `dist/index.js` and `dist/index.cjs`, and `recordTerminal`'s docblock reaches `dist/index.d.ts` — measured on the built tree against this package's own `files[]`, with a pre-existing description as the positive control and the module-private docblock (0 hits, comments stripped) as the negative one. Published bytes move, so this is a `patch`, not `skip-changeset`. Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 57fb801 commit a36b526

4 files changed

Lines changed: 280 additions & 10 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
`sys_automation_run.variables_json` states its presence discriminator in ONE direction, and a row-rebuilt snapshot no longer claims its steps are the pause's
6+
7+
Three corrections to text this package ships. No behaviour changes; every shape
8+
described below is the ruled design, measured as it already is.
9+
10+
**`variables_json` said `` where only `` holds.** The field description
11+
declared "present on a completed/failed row" and "the row's run had a pause its
12+
resume consumed before a downstream node failed" to be equivalent. The forward
13+
direction holds — nothing but the consumed-suspension path writes that column on
14+
a terminal row. The reverse does not, for one shape: a run that stranded, was
15+
restored and then finished. `recordTerminal` upserts the SAME `run_<id>` row
16+
with all four snapshot columns explicitly `null` — deliberately, so
17+
"restorable" cannot outlive the condition it describes — which leaves that row
18+
equal, across every column the discriminator is read from, to the row of a run
19+
that never paused at all. Absence means "nothing to restore now", never "this
20+
run never had one", and the restore verb already refuses in exactly those terms:
21+
it names the status it observed and declines to say which. The description now
22+
says so.
23+
24+
**A snapshot rebuilt from a row does not carry the step log as of the pause.**
25+
`deserializeConsumedSuspension`'s docblock said its `steps` are the log "AS OF
26+
THE PAUSE". That is true of the engine's process-local journal copy only, which
27+
slices `run.steps` back to the step count at the pause; the trimmed array is
28+
never persisted. `steps` are the one field the rebuild takes from the row's own
29+
`steps_json`, which is the terminal row's log of the WHOLE run — and both bounds
30+
on that column keep the failure on purpose (history compaction retains every
31+
failure; the byte cap trims the head). A row-rebuilt snapshot therefore carries
32+
steps the pause did not have. It re-arms the same run regardless: the pause is
33+
`nodeId` plus `variables` / `context` / `correlation`, none of which the step log
34+
feeds.
35+
36+
**`recordTerminal` now names the verb that reads what it writes** — the
37+
restore path in `engine.ts` — and the three properties of the write that are
38+
that verb's inputs rather than local detail. Its summary line also said
39+
"completed / failed" where the terminal vocabulary has had four members since
40+
the fold was removed from both ends of this write.
41+
42+
Both falsifying shapes are pinned in `suspended-run-store.test.ts`, including the
43+
indistinguishability itself: the restored-then-finished row and a never-paused
44+
row compare equal across those five columns, with the same comparison separating
45+
them while the snapshot is still there.

packages/services/service-automation/src/suspended-run-store.test.ts

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,182 @@ describe('ObjectStoreSuspendedRunStore — run-history retention + durable detai
467467
});
468468
});
469469

470+
// ─── What a terminal row does NOT say (#15336) ───────────────────────────────
471+
472+
/**
473+
* [#15336] Two declaration-truth residues of #13909 / #13937, each pinned by
474+
* the SHAPE that falsifies the sentence rather than by the sentence.
475+
*
476+
* 1. ⭐ `variables_json`'s presence discriminator holds ONE WAY. The object
477+
* declared presence and "this run had a pause its resume consumed" as
478+
* equivalent; the reverse fails for a run that stranded, was restored and
479+
* then finished. `recordTerminal` upserts the SAME `run_<id>` row with all
480+
* four snapshot columns explicitly NULL — deliberately, so "restorable"
481+
* cannot outlive the condition — which leaves that row byte-identical, in
482+
* those columns, to the row of a run that never paused at all. The second
483+
* test measures that indistinguishability directly, so the claim is not
484+
* "the column is null" but "no reader can recover the difference".
485+
* 2. ⭐ A snapshot rebuilt FROM A ROW does not carry the step log as of the
486+
* pause. `steps` are the one field `deserializeConsumedSuspension` takes
487+
* from the row's own `steps_json` rather than from the snapshot, and that
488+
* column holds the WHOLE run's compacted log — the failed attempt
489+
* included. Only the engine's process-local journal trims back to the step
490+
* count at the pause, and that trimmed array is never persisted.
491+
*
492+
* ⛔ Neither is a behaviour change: both shapes are the ruled design (a
493+
* restored run MUST clear what it carried; the terminal row's step log MUST
494+
* keep the failure). What was wrong was the text, and these pins are what stop
495+
* the corrected text from decaying back.
496+
*/
497+
describe('ObjectStoreSuspendedRunStore — what a terminal row does NOT say (#15336)', () => {
498+
/** The pause a strand's resume consumed — two steps, as of the pause. */
499+
const pauseSnapshot = (): SuspendedRun => ({
500+
runId: 'strandy',
501+
flowName: 'busy_flow',
502+
nodeId: 'hold',
503+
nodeType: 'hold',
504+
variables: { approved: true },
505+
steps: [
506+
{ nodeId: 'start', nodeType: 'start', status: 'success', startedAt: '2026-01-01T00:00:01.000Z' },
507+
{ nodeId: 'hold', nodeType: 'hold', status: 'success', startedAt: '2026-01-01T00:00:02.000Z' },
508+
],
509+
context: { object: 'crm_deal', tenantId: 'org_1' } as any,
510+
startedAt: '2026-01-01T00:00:00.000Z',
511+
startTime: 1735689600000,
512+
correlation: 'areq_9',
513+
});
514+
515+
/** The step log `recordLog` hands the store: the pause's steps AND the
516+
* failed attempt that ran after the resume consumed it. */
517+
const wholeRunSteps = (): RunRecord['steps'] => [
518+
...pauseSnapshot().steps,
519+
{
520+
nodeId: 'tail', nodeType: 'script', status: 'failure',
521+
startedAt: '2026-01-01T00:00:03.000Z',
522+
error: { code: 'TAIL_FAILED', message: 'tail exploded' },
523+
},
524+
];
525+
526+
/** The strand: a resume consumed the pause, then a downstream node threw. */
527+
const strandRecord = (): RunRecord => ({
528+
runId: 'strandy',
529+
flowName: 'busy_flow',
530+
status: 'failed',
531+
startedAt: '2026-01-01T00:00:00.000Z',
532+
error: 'tail exploded',
533+
// The PAUSE node, not the node that threw (#13937).
534+
nodeId: 'hold',
535+
steps: wholeRunSteps(),
536+
consumedSuspension: pauseSnapshot(),
537+
});
538+
539+
it('writes the snapshot on the strand, then CLEARS all four columns when the restored run finishes', async () => {
540+
const engine = createFakeEngine();
541+
const store = new ObjectStoreSuspendedRunStore(engine, createTestLogger());
542+
543+
await store.recordTerminal(strandRecord());
544+
const stranded = engine.rows.get('run_strandy');
545+
// Positive control for the nulls below: the row really did carry a
546+
// restorable snapshot, in all four columns, before the second write.
547+
expect(typeof stranded.variables_json).toBe('string');
548+
expect(stranded.node_type).toBe('hold');
549+
expect(stranded.correlation).toBe('areq_9');
550+
expect((await store.loadTerminal('strandy'))!.consumedSuspension).toBeDefined();
551+
552+
// …an operator restores it, the run resumes and this time it finishes.
553+
await store.recordTerminal({
554+
runId: 'strandy',
555+
flowName: 'busy_flow',
556+
status: 'completed',
557+
startedAt: '2026-01-01T00:00:00.000Z',
558+
steps: wholeRunSteps(),
559+
});
560+
561+
// The SAME row — an upsert, not a second history row.
562+
expect(engine.rows.size).toBe(1);
563+
const finished = engine.rows.get('run_strandy');
564+
expect(finished.variables_json).toBeNull();
565+
expect(finished.context_json).toBeNull();
566+
expect(finished.screen_json).toBeNull();
567+
expect(finished.node_type).toBeNull();
568+
expect(finished.correlation).toBeNull();
569+
570+
const reread = (await store.loadTerminal('strandy'))!;
571+
expect(reread.status).toBe('completed');
572+
expect(reread.consumedSuspension).toBeUndefined();
573+
// Not the drop notice either: the snapshot was not too big, it is gone
574+
// because the run that carried it moved on.
575+
expect(reread.consumedSuspensionDropped).toBeUndefined();
576+
});
577+
578+
it('⛔ leaves that row indistinguishable from a run that NEVER paused — the direction the column cannot answer', async () => {
579+
const engine = createFakeEngine();
580+
const store = new ObjectStoreSuspendedRunStore(engine, createTestLogger());
581+
582+
// A run that stranded, was restored, and finished.
583+
await store.recordTerminal(strandRecord());
584+
await store.recordTerminal({
585+
runId: 'strandy', flowName: 'busy_flow', status: 'completed',
586+
startedAt: '2026-01-01T00:00:00.000Z', steps: wholeRunSteps(),
587+
});
588+
// A run that never paused at all, same flow, same shape of log.
589+
await store.recordTerminal({
590+
runId: 'plain', flowName: 'busy_flow', status: 'completed',
591+
startedAt: '2026-01-01T00:00:00.000Z', steps: wholeRunSteps(),
592+
});
593+
594+
const columns = (id: string) => {
595+
const row = engine.rows.get(id);
596+
return {
597+
variables_json: row.variables_json, context_json: row.context_json,
598+
screen_json: row.screen_json, node_type: row.node_type, correlation: row.correlation,
599+
};
600+
};
601+
// The whole finding, in one assertion: the row of a run that DID have a
602+
// consumed suspension and the row of one that never had any are equal
603+
// across every column the discriminator is read from.
604+
expect(columns('run_strandy')).toEqual(columns('run_plain'));
605+
// Firing control — the same comparison DOES separate them while the
606+
// snapshot is still on the row, so the equality above is a measurement
607+
// and not a comparison of two things this helper cannot see.
608+
const engine2 = createFakeEngine();
609+
const store2 = new ObjectStoreSuspendedRunStore(engine2, createTestLogger());
610+
await store2.recordTerminal(strandRecord());
611+
await store2.recordTerminal({
612+
runId: 'plain', flowName: 'busy_flow', status: 'completed',
613+
startedAt: '2026-01-01T00:00:00.000Z', steps: wholeRunSteps(),
614+
});
615+
const columns2 = (id: string) => {
616+
const row = engine2.rows.get(id);
617+
return {
618+
variables_json: row.variables_json, context_json: row.context_json,
619+
screen_json: row.screen_json, node_type: row.node_type, correlation: row.correlation,
620+
};
621+
};
622+
expect(columns2('run_strandy')).not.toEqual(columns2('run_plain'));
623+
});
624+
625+
it('rebuilds a row snapshot with the WHOLE run\'s step log — ⛔ not the steps as of the pause', async () => {
626+
const engine = createFakeEngine();
627+
const store = new ObjectStoreSuspendedRunStore(engine, createTestLogger());
628+
await store.recordTerminal(strandRecord());
629+
630+
const rebuilt = (await store.loadTerminal('strandy'))!.consumedSuspension!;
631+
// Positive control: the rebuild IS the snapshot's — every field that
632+
// comes from the four snapshot columns round-trips.
633+
expect(rebuilt.nodeId).toBe('hold');
634+
expect(rebuilt.nodeType).toBe('hold');
635+
expect(rebuilt.correlation).toBe('areq_9');
636+
expect(rebuilt.variables).toEqual({ approved: true });
637+
638+
// `steps` alone do not: they are the terminal row's log, so the failed
639+
// attempt the pause never saw comes back inside the "snapshot".
640+
expect(rebuilt.steps).toEqual(wholeRunSteps());
641+
expect(rebuilt.steps).not.toEqual(pauseSnapshot().steps);
642+
expect(rebuilt.steps.map((s) => s.nodeId)).toContain('tail');
643+
});
644+
});
645+
470646
// ─── Trigger attribution columns (#7533) ─────────────────────────────────────
471647
//
472648
// The defect was information dropped ON THE WAY TO THE ROW, so these assertions

packages/services/service-automation/src/suspended-run-store.ts

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -624,11 +624,33 @@ export class ObjectStoreSuspendedRunStore implements SuspendedRunStore {
624624
}
625625

626626
/**
627-
* Persist a TERMINAL run (completed / failed) as durable history. Keyed by a
628-
* `run_`-prefixed id so it NEVER collides with a live suspended run's row
629-
* (id = raw `runId`, status `paused`) — the suspend save/load/delete/list
630-
* path (which only touches raw ids and `status:'paused'` rows) is untouched.
631-
* Upsert so a re-emitted terminal (e.g. a resumed run) updates in place.
627+
* Persist a TERMINAL run as durable history — whichever member of the one
628+
* terminal vocabulary the run reached ({@link isTerminalStatus}); ⛔ not
629+
* completed/failed only, which is the two-member fold #15223 removed from
630+
* both ends of this write. Keyed by a `run_`-prefixed id so it NEVER collides
631+
* with a live suspended run's row (id = raw `runId`, status `paused`) — the
632+
* suspend save/load/delete/list path (which only touches raw ids and
633+
* `status:'paused'` rows) is untouched. Upsert so a re-emitted terminal
634+
* (e.g. a resumed run) updates in place.
635+
*
636+
* ⭐ [#15336] The other end of this write lives in `engine.ts`: the row is
637+
* what `AutomationEngine.restoreConsumedSuspension` reads — through
638+
* {@link ObjectStoreSuspendedRunStore.loadTerminal} and
639+
* {@link deserializeConsumedSuspension} — when an operator asks for a
640+
* stranded run to be put back on its pause, and it is the only witness any
641+
* OTHER replica (or this one after a restart) has. Three things below are
642+
* that verb's inputs rather than local detail, so ⛔ do not change one
643+
* without reading it:
644+
*
645+
* - the four consumed-suspension columns are ALWAYS written, `null`
646+
* included — a restored run that later finishes must CLEAR what it
647+
* carried, which is also why an absent snapshot never means the run never
648+
* had one (`sys-automation-run.object.ts` states that direction at
649+
* `variables_json`);
650+
* - `node_id` carries the PAUSE node on a stranded row, because the
651+
* snapshot's node is rebuilt from that one column;
652+
* - an over-budget snapshot is recorded as a drop notice rather than
653+
* dropped silently, because a bare NULL reads as "the run moved on".
632654
*/
633655
async recordTerminal(record: RunRecord): Promise<void> {
634656
const now = new Date().toISOString();
@@ -1007,10 +1029,23 @@ function serializeConsumedSuspension(
10071029
*
10081030
* Keyed off `variables_json`, which no other terminal-row writer populates —
10091031
* see the call site. `correlation` and `node_type` come back from their own
1010-
* columns, written by the same helper. `steps` come from the row's own `steps_json`: they are the
1011-
* step log AS OF THE PAUSE (the engine trims the failed attempt's steps off the
1012-
* snapshot before recording), bounded by the same cap every terminal row's
1013-
* steps are.
1032+
* columns, written by the same helper.
1033+
*
1034+
* ⚠️ [#15336] `steps`, alone among them, do NOT come from the snapshot: they
1035+
* come from the row's own `steps_json`, which is the TERMINAL row's step log
1036+
* and covers the WHOLE run — the steps before the pause and the failed
1037+
* attempt's after it. Only the JOURNAL copy is as of the pause: the engine
1038+
* slices `run.steps` back to the step count at the pause when it journals a
1039+
* strand (`AutomationEngine.journalConsumedSuspension`), and that trimmed array
1040+
* never reaches this column — `recordTerminal` writes `record.steps`, the
1041+
* compacted log of the whole run, and compaction keeps every FAILURE on purpose
1042+
* while {@link serializeStepsBounded} trims the HEAD when the row is over
1043+
* budget, so the failed attempt's steps are the ones that survive both bounds.
1044+
* A snapshot rebuilt HERE therefore carries steps the pause did not have. It
1045+
* re-arms the same run regardless — the pause is `nodeId` plus `variables` /
1046+
* `context` / `correlation`, none of which the step log feeds — but ⛔ never
1047+
* read a row-rebuilt snapshot's `steps` as the state at the pause; the
1048+
* process-local journal is the only copy that is.
10141049
*/
10151050
function deserializeConsumedSuspension(row: any): SuspendedRun | undefined {
10161051
if (row.variables_json == null || row.variables_json === '') return undefined;

packages/services/service-automation/src/sys-automation-run.object.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,10 +269,24 @@ export const SysAutomationRun = ObjectSchema.create({
269269
// `RunRecord.consumedSuspensionDropped`), so "present" now has two shapes
270270
// and only one of them is a restorable snapshot — stated in the
271271
// description rather than left to the reader of the column.
272+
//
273+
// [#15336] The description used to state that presence and "this run had a
274+
// consumed suspension" imply EACH OTHER. Only the forward direction is
275+
// true. The falsifying shape is a run that stranded, was restored and then
276+
// finished: `recordTerminal` upserts the SAME `run_<id>` row and
277+
// `serializeConsumedSuspension(undefined)` writes explicit NULLs into all
278+
// four columns — deliberately, so "restorable" cannot outlive the condition
279+
// it describes (`RunRecord.consumedSuspension` in `engine.ts` states the
280+
// clearing rule from the producer's side). ⛔ The fix is the TEXT, not the
281+
// clearing: nothing reads the reverse direction — `restoreConsumedSuspension`
282+
// refuses a snapshot-less row by naming the status it observed and
283+
// deliberately declines to say whether the run never paused or lost its
284+
// snapshot, which is that direction being unavailable, honestly. The
285+
// clearing itself is pinned in `suspended-run-store.test.ts`.
272286
variables_json: Field.textarea({
273287
label: 'Variables',
274288
required: false,
275-
description: 'JSON snapshot of the flow variable map at suspend time. On a terminal row its PRESENCE is the discriminator: nothing but the consumed-suspension path writes it there, so variables_json present on a completed/failed row ⇔ the row\'s run had a pause that its resume consumed before a downstream node failed — the store\'s deserializer keys off exactly this. Two shapes on such a row: the snapshot itself (a restorable suspension), or a one-key notice `{"$consumedSuspensionDropped": …}` recording that the snapshot existed and was NOT persisted (over the store\'s row budget) — the notice is not a restorable snapshot; such a run can be restored only by the process that stranded it, while it runs.',
289+
description: 'JSON snapshot of the flow variable map at suspend time. On a terminal row its PRESENCE is the discriminator: nothing but the consumed-suspension path writes it there, so variables_json present on a completed/failed row ⇒ the row\'s run had a pause that its resume consumed before a downstream node failed — the store\'s deserializer keys off exactly this. ⛔ It does NOT hold in reverse: the terminal write is an upsert that always writes these columns, NULL included, so a stranded run that was restored and then finished has its row rewritten with NULLs. Absence therefore means "nothing to restore NOW", never "this run never had a consumed suspension" — the restore verb says exactly that when it refuses, naming the status it observed instead of claiming which. Two shapes on such a row: the snapshot itself (a restorable suspension), or a one-key notice `{"$consumedSuspensionDropped": …}` recording that the snapshot existed and was NOT persisted (over the store\'s row budget) — the notice is not a restorable snapshot; such a run can be restored only by the process that stranded it, while it runs.',
276290
group: 'State',
277291
}),
278292

0 commit comments

Comments
 (0)