From d773d7f9ed2a2b92dc55e40e43b857d7ec4e6af8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 13:07:21 +0000 Subject: [PATCH 1/7] wip: rescue in-progress work before container restart loss (flows) --- packages/runtime/src/domains/automation.ts | 7 + .../builtin/screen-resume-validation.test.ts | 270 ++++++++++++++++++ .../services/service-automation/src/engine.ts | 102 ++++++- .../src/screen-input-contract.ts | 137 +++++++++ .../spec/src/contracts/automation-service.ts | 11 +- 5 files changed, 525 insertions(+), 2 deletions(-) create mode 100644 packages/services/service-automation/src/builtin/screen-resume-validation.test.ts create mode 100644 packages/services/service-automation/src/screen-input-contract.ts diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 8f17d18f7d..4336c5bf61 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -321,6 +321,10 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // PERMISSION_DENIED → 403, the suspension is service-owned (#3801) // INVALID_SIGNAL → 400, the signal wrote the engine's `$` variable // namespace (#3853 follow-up) + // INVALID_SCREEN_INPUT → 400, the bag violates the suspended screen's + // declared field contract — a required field the + // caller was asked for is missing, or an + // undeclared key was sent (#4477) // RUN_NOT_FOUND → 404, no such suspension — unresumable for good // STORE_UNAVAILABLE → 503, the durable store is unreadable, so // existence is unknown; the same call is expected @@ -345,6 +349,9 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str if (result?.success === false && result.code === 'INVALID_SIGNAL') { return { handled: true, response: deps.error(result.error ?? 'Invalid resume signal', 400) }; } + if (result?.success === false && result.code === 'INVALID_SCREEN_INPUT') { + return { handled: true, response: deps.error(result.error ?? 'Invalid screen input', 400) }; + } if (result?.success === false && result.code === 'RUN_NOT_FOUND') { return { handled: true, response: deps.error(result.error ?? 'No such suspended run', 404) }; } diff --git a/packages/services/service-automation/src/builtin/screen-resume-validation.test.ts b/packages/services/service-automation/src/builtin/screen-resume-validation.test.ts new file mode 100644 index 0000000000..ea036b5a8a --- /dev/null +++ b/packages/services/service-automation/src/builtin/screen-resume-validation.test.ts @@ -0,0 +1,270 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `resume` enforces the suspended screen's declared field contract (#4477). + * + * The specimen is the issue's, verbatim: a two-field screen whose second field + * is `required` and conditional on the first. The RENDER half already worked — + * the paused result and `GET …/runs/:runId/screen` carry `required` and + * `visibleWhen` intact. There was no validation half: `POST …/resume` folded + * whatever bag it was handed straight into the flow variables, so all four + * shapes below completed the run with `success: true`. A client that skipped + * the dialog and posted to `resume` directly was unconstrained by every + * `required` the author declared. + * + * Screen flows are the one place where the declared field contract is the ONLY + * contract — no object schema sits behind a screen node to catch a bad bag + * downstream, unlike action params (ADR-0104 D2), record writes (ADR-0113) and + * approval `decisionOutputs` (#3447), all of which already enforce theirs. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { AutomationEngine } from '../engine.js'; +import { installBuiltinNodes } from './index.js'; + +function silentLogger() { + return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any; +} +function ctx() { + return { logger: silentLogger(), getService() { return undefined; } } as any; +} + +/** The issue's specimen: `kind` unconditionally required, `escalation_reason` + * required only when `kind == 'escalate'`. */ +function triageFlow() { + return { + name: 'triage', + label: 'Triage', + type: 'screen', + status: 'active', + version: 1, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'ask', type: 'screen', label: 'Triage', + config: { + fields: [ + { name: 'kind', label: 'Kind', type: 'text', required: true }, + { + name: 'escalation_reason', label: 'Escalation reason', type: 'text', + required: true, visibleWhen: "kind == 'escalate'", + }, + ], + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'ask', type: 'default' }, + { id: 'e2', source: 'ask', target: 'end', type: 'default' }, + ], + }; +} + +describe('screen resume validation (#4477)', () => { + let engine: AutomationEngine; + + beforeEach(() => { + engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, ctx()); + engine.registerFlow('triage', triageFlow() as any); + }); + + /** Run to the screen pause and return the run id. */ + async function pause(): Promise { + const started = await engine.execute('triage', {} as any); + expect(started.status).toBe('paused'); + expect(started.screen?.nodeId).toBe('ask'); + return started.runId!; + } + + it('still accepts the conforming submission — the conditional field stays hidden', async () => { + const runId = await pause(); + const res = await engine.resume(runId, { variables: { kind: 'normal' } }); + expect(res.success).toBe(true); + expect(res.code).toBeUndefined(); + }); + + it('accepts the conditional field when its predicate is TRUE and the value is there', async () => { + const runId = await pause(); + const res = await engine.resume(runId, { + variables: { kind: 'escalate', escalation_reason: 'customer churn risk' }, + }); + expect(res.success).toBe(true); + }); + + // ── The four rows of the issue's table ──────────────────────────────── + + it('rejects a VISIBLE conditional field whose required value is missing', async () => { + const runId = await pause(); + const res = await engine.resume(runId, { variables: { kind: 'escalate' } }); + expect(res.success).toBe(false); + expect(res.code).toBe('INVALID_SCREEN_INPUT'); + expect(res.error).toContain('escalation_reason'); + expect(res.error).toMatch(/required/i); + }); + + it('rejects a missing UNCONDITIONAL required field', async () => { + const runId = await pause(); + const res = await engine.resume(runId, { variables: {} }); + expect(res.success).toBe(false); + expect(res.code).toBe('INVALID_SCREEN_INPUT'); + expect(res.error).toContain('kind'); + }); + + it('rejects an UNDECLARED key', async () => { + const runId = await pause(); + const res = await engine.resume(runId, { variables: { kind: 'normal', totally_bogus: 'x' } }); + expect(res.success).toBe(false); + expect(res.code).toBe('INVALID_SCREEN_INPUT'); + expect(res.error).toContain('totally_bogus'); + // The declared list rides along, so the caller can self-correct — the + // same courtesy `decisionOutputs` extends (#3447). + expect(res.error).toContain("'kind'"); + expect(res.error).toContain("'escalation_reason'"); + }); + + it('treats an empty string / null as absent, not as a value', async () => { + for (const value of ['', ' ', null]) { + const runId = await pause(); + const res = await engine.resume(runId, { variables: { kind: value } }); + expect(res.success, `kind=${JSON.stringify(value)}`).toBe(false); + expect(res.code).toBe('INVALID_SCREEN_INPUT'); + } + }); + + // ── The refusal must not consume the pause ──────────────────────────── + + it('leaves the run resumable after a refusal — the pause is never consumed', async () => { + const runId = await pause(); + const bad = await engine.resume(runId, { variables: {} }); + expect(bad.success).toBe(false); + // The screen is still fetchable… + expect(engine.getSuspendedScreen(runId)?.nodeId).toBe('ask'); + // …and the legitimate submission still lands. + const good = await engine.resume(runId, { variables: { kind: 'normal' } }); + expect(good.success).toBe(true); + }); + + it('reports EVERY violation at once, not just the first', async () => { + const runId = await pause(); + const res = await engine.resume(runId, { variables: { escalation_reason: 'x', bogus: 1 } }); + expect(res.success).toBe(false); + // `kind` missing (required, unconditional) AND `bogus` undeclared. + expect(res.error).toContain('kind'); + expect(res.error).toContain('bogus'); + }); +}); + +describe('screen resume validation — screens that declare no contract (#4477)', () => { + function flowWith(nodeConfig: Record) { + return { + name: 'no_contract', label: 'No contract', type: 'screen', status: 'active', version: 1, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'ask', type: 'screen', label: 'Ask', config: nodeConfig }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'ask', type: 'default' }, + { id: 'e2', source: 'ask', target: 'end', type: 'default' }, + ], + }; + } + + it('leaves a MESSAGE-ONLY screen a pass-through — it declares no keys, so it constrains none', async () => { + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, ctx()); + engine.registerFlow('no_contract', flowWith({ title: 'Confirm', waitForInput: true }) as any); + const started = await engine.execute('no_contract', {} as any); + expect(started.status).toBe('paused'); + const res = await engine.resume(started.runId!, { variables: { acknowledged: true } }); + expect(res.success).toBe(true); + }); + + it('leaves an OBJECT-FORM screen a pass-through — the client persists the record and resumes with the id', async () => { + // Its `fields` is `[]` by construction; validating against that would + // reject the `idVariable` binding as an undeclared key, and the object's + // own `required` fields are enforced on the write path (ADR-0113). + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, ctx()); + engine.registerFlow('no_contract', flowWith({ + objectName: 'crm_account', mode: 'create', idVariable: 'account_id', + }) as any); + const started = await engine.execute('no_contract', {} as any); + expect(started.status).toBe('paused'); + expect(started.screen?.kind).toBe('object-form'); + const res = await engine.resume(started.runId!, { variables: { account_id: 'acc_1' } }); + expect(res.success).toBe(true); + }); +}); + +describe('screen resume validation — visibleWhen edge cases (#4477)', () => { + /** A conditional-required field whose predicate references a variable that + * is neither submitted nor in the run's snapshot. */ + function flowWithPredicate(visibleWhen: string) { + return { + name: 'pred', label: 'Pred', type: 'screen', status: 'active', version: 1, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'ask', type: 'screen', label: 'Ask', + config: { fields: [{ name: 'note', label: 'Note', type: 'text', required: true, visibleWhen }] }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'ask', type: 'default' }, + { id: 'e2', source: 'ask', target: 'end', type: 'default' }, + ], + }; + } + + async function resumeWith(visibleWhen: string, bag: Record) { + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, ctx()); + engine.registerFlow('pred', flowWithPredicate(visibleWhen) as any); + const started = await engine.execute('pred', {} as any); + return engine.resume(started.runId!, { variables: bag }); + } + + it('does not fire `required` for a field whose predicate is FALSE', async () => { + expect((await resumeWith('false', {})).success).toBe(true); + }); + + it('fires `required` for a field whose predicate is TRUE', async () => { + const res = await resumeWith('true', {}); + expect(res.success).toBe(false); + expect(res.code).toBe('INVALID_SCREEN_INPUT'); + }); + + it('does NOT reject when the predicate cannot be evaluated — the client decides what was shown', async () => { + // An unevaluable predicate is not evidence the field was on screen, so + // treating it as visible would reject a submission the user could never + // have completed — #3528's dead-end, moved server-side. Logged loudly + // instead (see `refuseInvalidScreenInput`). + const res = await resumeWith('nonexistent_var.deep.path == 1', {}); + expect(res.success).toBe(true); + }); + + it('still refuses an undeclared key on a screen whose predicate is unevaluable', async () => { + // The `required` degradation is scoped to `required` — the undeclared-key + // half of the contract does not depend on any predicate. + const res = await resumeWith('nonexistent_var.deep.path == 1', { note: 'x', rogue: 1 }); + expect(res.success).toBe(false); + expect(res.error).toContain('rogue'); + }); + + it('evaluates the predicate against the SUBMITTED values, not just the snapshot', async () => { + // `kind` exists only in this submission; the run's variable snapshot has + // nothing. A predicate resolved against the snapshot alone would read + // the field as hidden and let the missing value through. + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, ctx()); + engine.registerFlow('triage', triageFlow() as any); + const started = await engine.execute('triage', {} as any); + const res = await engine.resume(started.runId!, { variables: { kind: 'escalate' } }); + expect(res.success).toBe(false); + expect(res.error).toContain('escalation_reason'); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 937f825df3..f1ba8d86d3 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -9,8 +9,14 @@ import type { FlowFunctionEffect, FlowRunSummary, } from '@objectstack/spec/automation'; -import type { AutomationContext, AutomationResult, ResumeSignal, IAutomationService, ScreenSpec } from '@objectstack/spec/contracts'; +import type { AutomationContext, AutomationResult, ResumeSignal, IAutomationService, ScreenSpec, ScreenFieldSpec } from '@objectstack/spec/contracts'; import { RESUME_AUTHORITY_SERVICE } from '@objectstack/spec/contracts'; +import { + validateScreenInputs, + screenDeclaresInputContract, + declaredScreenFieldNames, + type ScreenFieldVisibility, +} from './screen-input-contract.js'; import type { Logger } from '@objectstack/spec/contracts'; import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, normalizeControlFlowRegions, collectFlowGraphs, findRegionEntry, defineActionDescriptor } from '@objectstack/spec/automation'; import { resolveFlowNodeExpressions } from '@objectstack/spec/automation'; @@ -2627,6 +2633,17 @@ export class AutomationEngine implements IAutomationService { } } + // The SCREEN contract (#4477). A run parked on a `screen` node + // declared exactly which keys it collects and which are required; + // until this ran, `resume` folded any bag at all straight into the + // variables, so a caller that skipped the dialog bypassed every + // `required` the author wrote. Checked here — beside the engine's + // other resume refusals and BEFORE the suspension is consumed — so + // a rejected bag leaves the pause live and the legitimate + // submission still lands. + const screenRefusal = this.refuseInvalidScreenInput(run, runId, signal); + if (screenRefusal) return screenRefusal; + // Restore the variable context and fold the signal in — the ONE // place a resume signal reaches the variable map. Runs BEFORE the // suspension is consumed, so a rejected signal changes nothing: @@ -2783,6 +2800,89 @@ export class AutomationEngine implements IAutomationService { } } + /** + * Enforce a suspended `screen` node's declared field contract against the + * submitted bag, returning a refusal or `null` to allow (#4477). + * + * The render half of `screen` always worked — the trigger response and + * `GET …/runs/:runId/screen` carry `required` and `visibleWhen` intact, so + * a renderer had everything it needed. There was no validation half: + * `resume` accepted `{}` on a screen with an unconditional `required` + * field, accepted a visible conditional field's value being absent, and + * accepted keys the screen never declared — every one of them completing + * the run. A client that skipped the dialog and posted here directly was + * unconstrained by anything the flow author wrote. + * + * Scope, and the reasons for each edge: + * + * - **Only `signal.variables`.** That is the screen's collected-values + * channel (the executor surfaces `fields`, the runner posts `inputs`). + * `signal.output` is the node-OUTPUT namespace, lands under + * `${nodeId}.${key}`, and belongs to the approval-style resume envelope + * — a different contract, not this one's to police. + * - **Only a screen that declares fields** — see + * {@link screenDeclaresInputContract}. An object-form screen and a + * message-only screen declare no keys, so they constrain none (the same + * pass-through `enforceActionParams` gives a param-less action). + * - **Never an engine-built signal.** The subflow output mapping and the + * `map` item handoff are the engine's own continuations; they carry + * author-named output variables, not a screen submission. + * + * `visibleWhen` is evaluated against the SUBMITTED values first (layered + * over the run's variables, so a predicate may reference a prior node), + * because a hidden field's `required` must not fire — that is #3528's + * dead-end reproduced server-side. An unevaluable predicate is reported and + * treated as hidden: the client decides what the user saw, and a broken + * predicate is not evidence a field was shown. + */ + private refuseInvalidScreenInput( + run: SuspendedRun, + runId: string, + signal: ResumeSignal | undefined, + ): AutomationResult | null { + if (!signal) return null; + if ((signal as Record)[ENGINE_BUILT_SIGNAL] === true) return null; + if (!screenDeclaresInputContract(run.screen)) return null; + const fields = run.screen!.fields; + + const bag = (signal.variables ?? {}) as Record; + // Submitted values win over the snapshot: the predicate is about what + // the user is filling in NOW, and the run's variables only supply the + // wider context a `visibleWhen` may legitimately reference. + const scope = new Map(Object.entries(run.variables)); + for (const [k, v] of Object.entries(bag)) scope.set(k, v); + + const visibility = (field: ScreenFieldSpec): ScreenFieldVisibility => { + try { + return this.evaluateCondition(String(field.visibleWhen), scope); + } catch (err) { + this.logger.warn( + `[automation] run '${runId}': screen field '${field.name}' has a visibleWhen that could not be ` + + `evaluated (\`${field.visibleWhen}\`: ${(err as Error)?.message}) — its \`required\` is not ` + + `enforced for this submission`, + ); + return undefined; + } + }; + + const issues = validateScreenInputs(fields, bag, visibility); + if (!issues.length) return null; + + const declared = declaredScreenFieldNames(fields); + const summary = issues.map((i) => i.message).join('; '); + this.logger.warn( + `[automation] refused resume of run '${runId}': screen '${run.nodeId}' input violates its declared ` + + `field contract — ${summary}`, + ); + return { + success: false, + code: 'INVALID_SCREEN_INPUT', + error: + `Invalid screen input: ${summary} — declared fields: ` + + `${declared.map((n) => `'${n}'`).join(', ') || '(none)'}`, + }; + } + /** * Build the resume signal that maps a completed subflow child's output * into its parent — mirroring the synchronous path exactly: the engine's diff --git a/packages/services/service-automation/src/screen-input-contract.ts b/packages/services/service-automation/src/screen-input-contract.ts new file mode 100644 index 0000000000..7df78e4b04 --- /dev/null +++ b/packages/services/service-automation/src/screen-input-contract.ts @@ -0,0 +1,137 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Screen-input VALUE contract (#4477). + * + * A `screen` node's `config.fields` is a complete input contract — the author + * declares the keys, their `required`-ness, and (via `visibleWhen`) when a + * field is even asked for. Before this module the contract informed the CLIENT + * dialog only: `POST …/runs/:runId/resume` folded whatever bag it was handed + * straight into the flow variables, so a caller that skipped the dialog and + * posted to `resume` directly bypassed every `required` the flow author wrote. + * Missing required fields and undeclared keys alike completed the run. + * + * Screen flows are the one place where the declared field contract is the ONLY + * contract — there is no object schema behind a screen node to catch a bad bag + * downstream. The platform enforces the analogous contract everywhere else this + * seam appears: action params (`validateActionParams`, ADR-0104 D2), record + * writes (ADR-0113), approval `decisionOutputs` (#3447). This is that rule for + * screen resume, deliberately built in the same shape — a PURE check returning + * issues, with the disposition (reject with a 400-worthy refusal) owned by the + * caller. + */ + +import type { ScreenFieldSpec, ScreenSpec } from '@objectstack/spec/contracts'; + +/** One violation of a screen's declared field contract. */ +export interface ScreenInputIssue { + /** The offending key — a declared field name, or the undeclared key sent. */ + field: string; + /** + * Which constraint the bag violated. Same two names the action-param + * contract uses (`required` / `unknown_field`, ADR-0114) so a client does not + * learn a second vocabulary for the same two conditions. + */ + code: 'required' | 'unknown_field'; + message: string; +} + +/** + * Visibility verdict for one conditional field. `true`/`false` are answers; + * `undefined` means the predicate could not be evaluated at all. + */ +export type ScreenFieldVisibility = boolean | undefined; + +function isPresent(v: unknown): boolean { + return v !== undefined && v !== null && !(typeof v === 'string' && v.trim() === ''); +} + +/** + * Whether a screen surfaces a field contract worth enforcing. + * + * Two screens deliberately declare NOTHING and so keep the historical + * pass-through, mirroring `enforceActionParams`' "an action with no `params` + * is untouched": + * + * - an **object-form** screen (`kind: 'object-form'`) — its `fields` is empty + * by construction because the CLIENT renders the object's own form, persists + * the record through the normal write path (which enforces the object's + * `required` fields itself, ADR-0113) and resumes with only the saved id + * bound to `idVariable`. There is no flat field list to validate against, + * and validating the bag against `[]` would reject that id as undeclared. + * - a **message-only** screen (`waitForInput: true`, no fields) — a + * confirmation step. It declares no keys, so it constrains none. + */ +export function screenDeclaresInputContract(screen: ScreenSpec | undefined): boolean { + if (!screen) return false; + if (screen.kind === 'object-form') return false; + return Array.isArray(screen.fields) && screen.fields.length > 0; +} + +/** + * Validate a submitted screen bag against the suspended node's declared fields. + * Returns the list of issues (empty ⇒ conformant). Does NOT throw. + * + * Enforced, and nothing beyond it: + * - `required` presence for every field the caller was actually asked for; + * - undeclared keys. + * + * `visibleWhen` is resolved FIRST, by the caller-supplied {@link visibility} + * probe, because a hidden field's `required` must not fire: the client never + * showed it, so demanding it would dead-end the run at Submit — the exact + * failure #3528 filed. A field whose predicate cannot be evaluated is treated + * as hidden (its `required` is not enforced) rather than visible: the client is + * the authority on what the user was shown, and an unevaluable predicate is not + * evidence the field was on screen. It is reported to the caller so the + * degradation is loud rather than silent. Its KEY stays accepted either way — + * the author declared it, so it is never "undeclared". + * + * Value SHAPE (`type`) is out of scope here: a screen field's `type` is a + * widget hint with no closed vocabulary, unlike an action param's field type. + */ +export function validateScreenInputs( + fields: readonly ScreenFieldSpec[], + bag: Record, + visibility: (field: ScreenFieldSpec) => ScreenFieldVisibility, +): ScreenInputIssue[] { + const issues: ScreenInputIssue[] = []; + const declared = new Map(); + for (const f of fields) if (f?.name) declared.set(f.name, f); + + for (const field of declared.values()) { + if (field.required !== true) continue; + if (isPresent(bag[field.name])) continue; + // Conditional field: only a predicate that evaluates TRUE makes `required` + // fire. `false` (hidden — not asked for) and `undefined` (unevaluable) both + // leave it alone. + if (field.visibleWhen != null && String(field.visibleWhen).trim() !== '') { + if (visibility(field) !== true) continue; + } + issues.push({ + field: field.name, + code: 'required', + message: `Screen field "${field.name}" is required`, + }); + } + + for (const key of Object.keys(bag)) { + if (declared.has(key)) continue; + issues.push({ + field: key, + code: 'unknown_field', + message: `Unknown screen field "${key}" — not declared on this screen`, + }); + } + + return issues; +} + +/** + * The declared field names, for an error that tells a caller what it MAY send — + * the same courtesy `decisionOutputs` already extends (#3447), and the + * difference between a rejection an agent can self-correct from and one it + * can only guess at. + */ +export function declaredScreenFieldNames(fields: readonly ScreenFieldSpec[]): string[] { + return fields.map((f) => f?.name).filter((n): n is string => typeof n === 'string' && n.length > 0); +} diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index 6b5d386c23..2ade00df40 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/packages/spec/src/contracts/automation-service.ts @@ -204,11 +204,20 @@ export interface AutomationResult { * running; this duplicate was refused so side effects cannot run twice. * A transport maps it to **409**. The other resume is doing the work, * so callers should treat it as benign. + * - `'INVALID_SCREEN_INPUT'` — the run is parked on a `screen` node and + * the submitted bag violates that screen's declared field contract: a + * `required` field the caller WAS asked for is missing, or a key the + * screen never declared was sent (#4477). A transport maps it to + * **400**. Distinct from `'INVALID_SIGNAL'`, which is about the + * engine's own `$` variable namespace rather than the author's field + * declarations. `visibleWhen` is evaluated against the submitted values + * first, so a HIDDEN field's `required` never fires — enforcing it would + * dead-end the run at a field the user was never shown (#3528). * * All of these refuse before consuming the suspension: the run stays parked * and the legitimate continuation still lands. */ - code?: 'PERMISSION_DENIED' | 'INVALID_SIGNAL' | 'RUN_NOT_FOUND' | 'STORE_UNAVAILABLE' | 'RESUME_IN_PROGRESS'; + code?: 'PERMISSION_DENIED' | 'INVALID_SIGNAL' | 'RUN_NOT_FOUND' | 'STORE_UNAVAILABLE' | 'RESUME_IN_PROGRESS' | 'INVALID_SCREEN_INPUT'; /** * Lifecycle status. `'paused'` means the run suspended at a node (e.g. * an Approval node awaiting a human decision, ADR-0019) and can be From 2e041d593ceb342a92f8bc46ac998ba73141958b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 13:11:08 +0000 Subject: [PATCH 2/7] fix(automation): validate screen resume input against the node's declared fields (#4477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changeset for the screen-resume field contract landed in the preceding commit (which the container restart forced out as a WIP rescue): `resume` now refuses a bag that violates the suspended screen's declared `fields` — a missing `required` value the caller WAS asked for, or a key the screen never declared — with the new `INVALID_SCREEN_INPUT` code, mapped to 400 by the automation domain route. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --- .../screen-resume-declared-field-contract.md | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 .changeset/screen-resume-declared-field-contract.md diff --git a/.changeset/screen-resume-declared-field-contract.md b/.changeset/screen-resume-declared-field-contract.md new file mode 100644 index 0000000000..78b441e368 --- /dev/null +++ b/.changeset/screen-resume-declared-field-contract.md @@ -0,0 +1,49 @@ +--- +"@objectstack/spec": patch +"@objectstack/service-automation": patch +"@objectstack/runtime": patch +--- + +fix(automation): `resume` enforces the suspended screen's declared field contract (#4477) + +A `screen` node's `config.fields` is a complete input contract — the author +declares the keys, their `required`-ness, and (via `visibleWhen`) when a field +is even asked for. The RENDER half honoured all of it: the paused result and +`GET …/runs/:runId/screen` carry `required` and `visibleWhen` intact. There was +no VALIDATION half — `POST …/runs/:runId/resume` folded whatever bag it was +handed straight into the flow variables, so a caller that skipped the dialog and +posted here directly was unconstrained by every `required` the author wrote. +Missing required fields, and keys the screen never declared, all completed the +run with `success: true`. + +Screen flows are the one place where the declared field contract is the ONLY +contract — no object schema sits behind a screen node to catch a bad bag +downstream. The platform already enforces the analogous contract everywhere else +this seam appears: action params (ADR-0104 D2), record writes (ADR-0113), +approval `decisionOutputs` (#3447). This is that rule for screen resume, built in +the same shape. + +`resume` now refuses a non-conforming submission with the new +`AutomationResult.code` `'INVALID_SCREEN_INPUT'` (a transport maps it to **400**, +as the automation domain route now does) and an `Invalid screen input: …` message +that names each violation and lists the declared field names. The refusal happens +BEFORE the suspension is consumed, so the pause stays live and the legitimate +submission still lands. + +`visibleWhen` is evaluated against the SUBMITTED values first (layered over the +run's variable snapshot), so a hidden field's `required` never fires — enforcing +it would dead-end the run at a field the user was never shown, which is #3528 +reproduced server-side. A predicate that cannot be evaluated is logged and +treated as hidden rather than visible: the client decides what the user saw, and +a broken predicate is not evidence a field was on screen. + +Scope, deliberately narrow — three shapes keep the historical pass-through: + +- an **object-form** screen (`kind: 'object-form'`), whose `fields` is empty by + construction because the client renders the object's own form and the write + path enforces that object's `required` fields itself; +- a **message-only** screen (`waitForInput: true`, no fields), which declares no + keys and so constrains none — the same pass-through `enforceActionParams` + gives a param-less action; +- `signal.output`, the node-OUTPUT namespace, which belongs to the approval-style + resume envelope rather than to the screen's collected-values channel. From f65866b68e688ce1b1802e9fef04a4a686b1e405 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 13:18:33 +0000 Subject: [PATCH 3/7] fix(approvals): record an admin override of a staffed approver slate as an override (#4466) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sys_approval_action` had no override column, so an admin overriding a properly staffed approver slate wrote a row byte-for-byte identical to the designated approver approving normally. A reader of the timeline saw `approve` by the admin and could not tell which had happened; the bypassed approver's `409 INVALID_STATE` was the only trace, and only if they happened to try. The platform knows at decision time — it took the `isOverrideActor` branch to admit the call — so this was dropped information, not unavailable information. Adds `sys_approval_action.via_override` (optional boolean), set on exactly the actions admitted by that branch: `decideNode`'s approve/reject and `reassign`'s admin rescue. Surfaced on `ApprovalActionRow`, returned by `listActions`, and added to `highlightFields` + two grid views. `true` = admitted only by the override branch. `false` = checked, not an override (an admin who IS a slot holder approves normally). Absent = a row predating the column — "not recorded" is not the claim "not an override", so `rowFromAction` maps `null` to `undefined`. Additive and nullable: no migration needed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --- .changeset/approval-override-audit-marker.md | 47 ++++ .../src/approval-override-audit.test.ts | 210 ++++++++++++++++++ .../plugin-approvals/src/approval-service.ts | 20 ++ .../src/sys-approval-action.object.ts | 32 ++- .../src/translations/en.objects.generated.ts | 4 + .../translations/es-ES.objects.generated.ts | 4 + .../translations/ja-JP.objects.generated.ts | 4 + .../translations/zh-CN.objects.generated.ts | 4 + .../spec/src/contracts/approval-service.ts | 19 ++ 9 files changed, 341 insertions(+), 3 deletions(-) create mode 100644 .changeset/approval-override-audit-marker.md create mode 100644 packages/plugins/plugin-approvals/src/approval-override-audit.test.ts diff --git a/.changeset/approval-override-audit-marker.md b/.changeset/approval-override-audit-marker.md new file mode 100644 index 0000000000..16dd713433 --- /dev/null +++ b/.changeset/approval-override-audit-marker.md @@ -0,0 +1,47 @@ +--- +"@objectstack/spec": patch +"@objectstack/plugin-approvals": patch +--- + +fix(approvals): record an admin override of a staffed approver slate AS an override (#4466) + +An admin who is not in a request's `pending_approvers` may still act on it — the +`#3424` privileged-override path exists so a request routed to an unstaffed +position, or to approvers who have all left, is not undecidable forever. The +override is defensible; what was not is what the audit trail recorded. + +`sys_approval_action` had no override column at all. So an admin overriding a +properly-staffed slate wrote a row **byte-for-byte identical** to the designated +approver approving normally: a reader of the timeline saw `approve` by the admin +and could not tell whether the admin *was* an approver or *overrode* the ones who +were, and the bypassed approver's later `409 INVALID_STATE` was the only trace — +existing only if they happened to try. The platform knows at decision time (it +took the `isOverrideActor` branch to admit the call at all), so this was dropped +information, not unavailable information. The whole point of an approval record +is to answer "who authorized this, and were they entitled to?". + +`sys_approval_action` now carries **`via_override`** (boolean, optional), set on +exactly the actions admitted by that branch — `decideNode`'s approve/reject and +`reassign`'s admin rescue. It is surfaced on `ApprovalActionRow.via_override` +(`@objectstack/spec/contracts`), returned by `listActions`, and added to the +object's `highlightFields` and two grid list views so a timeline can say +"overrode the approver slate" instead of rendering it as an ordinary approval. + +Three distinctions the column keeps apart deliberately: + +- **`true`** — the actor held no slot in the slate and was admitted only by the + override branch. +- **`false`** — checked, and it was not an override. An admin who *is* a + designated approver is approving normally and records `false`: the marker is + about which branch admitted the call, not about whether the actor holds admin + rights. +- **absent** — a row written before this column existed. "Not recorded" is not + the same claim as "not an override", so `rowFromAction` maps `null` to + `undefined` rather than to `false`. + +Additive and nullable, so this needs no data migration: existing rows keep +working and simply read as unrecorded. Levelled `patch` rather than `minor` +because nothing an author writes changes — but note it *is* an observable +behaviour change on a read surface: `listActions` responses and the +`sys_approval_action` grid views now carry a field consumers did not see before, +and `sys_approval_action` gains a column on next schema sync. diff --git a/packages/plugins/plugin-approvals/src/approval-override-audit.test.ts b/packages/plugins/plugin-approvals/src/approval-override-audit.test.ts new file mode 100644 index 0000000000..6896f0e680 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-override-audit.test.ts @@ -0,0 +1,210 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * An admin override of a STAFFED approver slate is recorded as an override (#4466). + * + * The reproduction is the issue's: `showcase_dynamic_approval` stage 2 resolved + * its `expression` approvers correctly to exactly one designated user, and the + * admin — who was not in that slate — approved it anyway through the #3424 + * privileged path. The designated approver then got `409 INVALID_STATE`. + * + * The override itself is defensible; what was not is the audit trail. Before + * this, `sys_approval_action` had no override column at all, so an admin + * overriding a properly-staffed slate wrote a row byte-for-byte identical to the + * designated approver approving normally. A reader of the timeline saw `approve` + * by the admin and could not tell which had happened, and the bypassed + * approver's 409 was the only trace — existing only if they happened to try. + * + * The platform KNOWS at decision time: it took the `isOverrideActor` branch to + * admit the call. This was dropped information, not unavailable information. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ApprovalService } from './approval-service.js'; + +interface FakeRow { [k: string]: any } + +/** The same minimal engine shape `approval-service.test.ts` uses. */ +function makeFakeEngine() { + const tables: Record = {}; + const ensure = (n: string) => (tables[n] ??= []); + function matches(row: FakeRow, filter: any): boolean { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or') { + if (!(v as any[]).some(sub => matches(row, sub))) return false; + continue; + } + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (rv !== v) return false; + } + return true; + } + return { + _tables: tables, + async find(object: string, options?: any) { + const rows = ensure(object).filter(r => matches(r, options?.filter ?? options?.where)); + if (options?.orderBy?.[0]) { + const { field, order } = options.orderBy[0]; + rows.sort((a, b) => { + const av = a[field]; const bv = b[field]; + if (av === bv) return 0; + const cmp = av > bv ? 1 : -1; + return order === 'desc' ? -cmp : cmp; + }); + } + const start = options?.offset ?? 0; + return rows.slice(start, start + (options?.limit ?? 1000)); + }, + async insert(object: string, data: any) { ensure(object).push({ ...data }); return { ...data }; }, + async update(object: string, idOrData: any, _opts?: any) { + const data = typeof idOrData === 'object' ? idOrData : _opts; + const id = typeof idOrData === 'object' ? idOrData.id : idOrData; + const table = ensure(object); + const i = table.findIndex(r => r.id === id); + if (i >= 0) table[i] = { ...table[i], ...data }; + return table[i]; + }, + async delete(object: string, options?: any) { + const table = ensure(object); + const i = table.findIndex(r => r.id === (options?.where?.id ?? options?.id)); + if (i >= 0) table.splice(i, 1); + return {}; + }, + registerHook() {}, unregisterHooksByPackage() { return 0; }, async fire() {}, + }; +} + +const SYS = { isSystem: true, positions: [], permissions: [] } as any; +const SUBMITTER = { userId: 'u1', tenantId: 't1', positions: [], permissions: [] } as any; +/** The designated approver the slate actually names. */ +const DESIGNATED = { userId: 'u9', tenantId: 't1', positions: [], permissions: [] } as any; +/** An admin who is NOT in the slate — the issue's actor. */ +const ADMIN = { userId: 'root', tenantId: 't1', positions: [], permissions: ['admin_full_access'] } as any; +/** An admin who IS a designated approver — approving normally, not overriding. */ +const ADMIN_ON_SLATE = { userId: 'u9', tenantId: 't1', positions: [], permissions: ['admin_full_access'] } as any; + +describe('approval override audit marker (#4466)', () => { + let engine: ReturnType; + let svc: ApprovalService; + let n = 0; + const baseTime = new Date('2026-01-15T10:00:00Z').getTime(); + + beforeEach(() => { + engine = makeFakeEngine(); + n = 0; + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(baseTime + (n++) * 1000) }, + }); + }); + + /** A request whose slate is properly STAFFED — one real, designated user. */ + const staffedInput = (extra: Record = {}) => ({ + object: 'opportunity', recordId: 'opp1', runId: 'run_1', nodeId: 'co_sign', + flowName: 'showcase_dynamic_approval', + config: { + approvers: [{ type: 'user' as const, value: 'u9' }], + behavior: 'first_response' as const, lockRecord: true, + }, + record: { id: 'opp1', amount: 100 }, + ...extra, + }); + + it('the repro: the slate names exactly one designated user, and the admin is not on it', async () => { + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + expect(req.pending_approvers).toEqual(['u9']); + expect(req.pending_approvers).not.toContain('root'); + }); + + it('marks an admin override of a STAFFED slate as `via_override`', async () => { + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'root', comment: 'not mine' }, ADMIN); + + const acts = await svc.listActions(req.id, SYS); + const decision = acts.at(-1)!; + expect(decision).toMatchObject({ action: 'approve', actor_id: 'root', comment: 'not mine' }); + // The bit the audit trail used to drop entirely. + expect(decision.via_override).toBe(true); + }); + + it('does NOT mark the designated approver’s own approval', async () => { + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, DESIGNATED); + + const decision = (await svc.listActions(req.id, SYS)).at(-1)!; + expect(decision).toMatchObject({ action: 'approve', actor_id: 'u9' }); + // An explicit `false`, not absent: "checked, and it was not an override" is + // a different claim from a legacy row's "not recorded". + expect(decision.via_override).toBe(false); + }); + + it('does NOT mark an admin who is ALSO a designated approver — that is an ordinary approval', async () => { + // The marker is about which BRANCH admitted the call, not about whether the + // actor happens to hold admin rights. Collapsing the two would make every + // admin's ordinary decision read as a slate bypass. + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, ADMIN_ON_SLATE); + + expect((await svc.listActions(req.id, SYS)).at(-1)!.via_override).toBe(false); + }); + + it('marks an override REJECTION too — the direction of the decision is irrelevant', async () => { + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + await svc.decideNode(req.id, { decision: 'reject', actorId: 'root' }, ADMIN); + + const decision = (await svc.listActions(req.id, SYS)).at(-1)!; + expect(decision).toMatchObject({ action: 'reject', via_override: true }); + }); + + it('the override and the ordinary approval are no longer identical rows', async () => { + // The issue's core claim, asserted directly: before the column, these two + // decisions differed only in `actor_id` — and an `actor_id` alone cannot + // answer "were they entitled to?". + const a = await svc.openNodeRequest(staffedInput(), SUBMITTER); + await svc.decideNode(a.id, { decision: 'approve', actorId: 'root' }, ADMIN); + const overrideRow = (await svc.listActions(a.id, SYS)).at(-1)!; + + const b = await svc.openNodeRequest( + staffedInput({ recordId: 'opp2', runId: 'run_2', record: { id: 'opp2', amount: 100 } }), + SUBMITTER, + ); + await svc.decideNode(b.id, { decision: 'approve', actorId: 'u9' }, DESIGNATED); + const normalRow = (await svc.listActions(b.id, SYS)).at(-1)!; + + expect(overrideRow.action).toBe(normalRow.action); + expect(overrideRow.via_override).not.toBe(normalRow.via_override); + }); + + it('marks an admin RESCUE reassign of a slate they hold no slot in', async () => { + // #3424's other privileged action: handing a stuck (or staffed) request to + // a real approver. Same fact, same column. + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + await svc.reassign(req.id, { actorId: 'root', to: 'u7' }, ADMIN); + + const row = (await svc.listActions(req.id, SYS)).at(-1)!; + expect(row).toMatchObject({ action: 'reassign', reassign_to: 'u7', via_override: true }); + }); + + it('does NOT mark a slot holder handing over their own slot', async () => { + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + await svc.reassign(req.id, { actorId: 'u9', to: 'u7' }, DESIGNATED); + + expect((await svc.listActions(req.id, SYS)).at(-1)!.via_override).toBe(false); + }); + + it('a legacy row (written before the column existed) reads as UNRECORDED, not as "not an override"', async () => { + const req = await svc.openNodeRequest(staffedInput(), SUBMITTER); + // Simulate a row persisted by an older build: the column is simply absent. + const rows = engine._tables['sys_approval_action']; + delete rows[rows.length - 1].via_override; + + const row = (await svc.listActions(req.id, SYS)).at(-1)!; + expect(row.via_override).toBeUndefined(); + expect(row.via_override).not.toBe(false); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 592ccbfcd6..b2876ea505 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -453,6 +453,11 @@ function rowFromAction(row: any): ApprovalActionRow { // Structured reassign hand-off parties (#4365). reassign_from: row.reassign_from ?? undefined, reassign_to: row.reassign_to ?? undefined, + // #4466 — surfaced so a timeline can SAY "overridden the approver slate" + // rather than render an override identically to an ordinary approval. + // `null` (a row written before the column existed) stays `undefined`: + // "not recorded" is not the same claim as "not an override". + via_override: row.via_override == null ? undefined : row.via_override === true, // Decision attachments (#3266): rich descriptors carrying the display name + // download URL, so consumers label/open them without reading `sys_file`. attachments: attachments.length ? attachments : undefined, @@ -1664,6 +1669,11 @@ export class ApprovalService implements IApprovalService { if (!isSlotHolder && !isOverride) { throw new Error(`FORBIDDEN: actor '${actorId}' is not a pending approver`); } + // #4466 — the audit fact this decision would otherwise drop: the actor was + // admitted ONLY by the override branch, holding no slot in the staffed + // slate. An admin who IS a slot holder is approving normally, so the two + // conditions are recorded apart rather than collapsed into "actor is admin". + const viaOverride = isOverride && !isSlotHolder; const config = parseJson(raw.node_config_json, { approvers: [], behavior: 'first_response' } as any); const org = raw.organization_id ?? null; @@ -1750,6 +1760,10 @@ export class ApprovalService implements IApprovalService { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: input.decision, actor_id: actorId, comment: input.comment ?? null, + // #4466: the override is recorded on the DECISION, not inferred later. + // Written as an explicit `false` for an ordinary decision so a reader can + // tell "checked, and it was not an override" from a legacy row's `null`. + via_override: viaOverride, attachments: input.attachments?.length ? input.attachments : null, created_at: now, }, { context: SYSTEM_CTX }); @@ -2405,6 +2419,11 @@ export class ApprovalService implements IApprovalService { } const isOverride = this.isOverrideActor(context, raw.organization_id ?? null); const from = String(input.from ?? actorId).trim(); + // #4466 — same rule as `decideNode`: the marker records that the actor was + // admitted only by the privileged branch, holding no slot themselves. A + // reassign is the other action #3424 lets an admin take over a slate they + // are not on, so it carries the same fact. + const viaOverride = isOverride && !pending.includes(actorId); let next: string[]; if (pending.includes(from)) { // Normal hand-off: the actor holds the slot being moved (or is a @@ -2431,6 +2450,7 @@ export class ApprovalService implements IApprovalService { // comment (`" → "`) baked raw user ids into user-facing text. // `comment` is pure user input: absent unless the actor wrote one. actor_id: actorId, reassign_from: from, reassign_to: to, + via_override: viaOverride, comment: input.comment ?? null, created_at: now, }, { context: SYSTEM_CTX }); // per_group / quorum (#3266): carry the delegated slot's group membership to diff --git a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts index cd7a01cd49..b0055d25bc 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts @@ -25,7 +25,7 @@ export const SysApprovalAction = ObjectSchema.create({ displayNameField: 'id', nameField: 'id', // [ADR-0079] canonical primary-title pointer (mirrors deprecated displayNameField) titleFormat: '{action} · {step_name}', - highlightFields: ['request_id', 'step_name', 'action', 'actor_id', 'created_at'], + highlightFields: ['request_id', 'step_name', 'action', 'actor_id', 'via_override', 'created_at'], // ADR-0104 D3 wave 2. `attachments` is a media field, so the files it holds // are OWNED by this row — and the storage service would otherwise authorize @@ -42,7 +42,7 @@ export const SysApprovalAction = ObjectSchema.create({ name: 'recent', label: 'Recent', data: { provider: 'object', object: 'sys_approval_action' }, - columns: ['created_at', 'request_id', 'step_name', 'action', 'actor_id', 'comment'], + columns: ['created_at', 'request_id', 'step_name', 'action', 'actor_id', 'via_override', 'comment'], sort: [{ field: 'created_at', order: 'desc' }], pagination: { pageSize: 50 }, emptyState: { title: 'No approval actions yet', message: 'Actions are logged automatically when approvals progress.' }, @@ -62,7 +62,7 @@ export const SysApprovalAction = ObjectSchema.create({ name: 'all_actions', label: 'All', data: { provider: 'object', object: 'sys_approval_action' }, - columns: ['created_at', 'request_id', 'step_name', 'action', 'actor_id', 'comment'], + columns: ['created_at', 'request_id', 'step_name', 'action', 'actor_id', 'via_override', 'comment'], sort: [{ field: 'created_at', order: 'desc' }], pagination: { pageSize: 100 }, }, @@ -119,6 +119,32 @@ export const SysApprovalAction = ObjectSchema.create({ comment: Field.textarea({ label: 'Comment', required: false, group: 'Action' }), + // #4466 — the one bit of "who really decided this" that was still dropped. + // A privileged admin may act on a request whose staffed approver slate they + // hold no slot in (the #3424 override path); before this column, that + // decision was byte-for-byte identical to the designated approver's own + // approval. A reader of the timeline saw `approve` by the admin and could + // not tell whether the admin WAS an approver or OVERRODE the ones who were, + // and the bypassed approver's later `409 INVALID_STATE` was the only trace + // — existing only if they happened to try. + // + // The platform KNOWS at decision time: it took the `isOverrideActor` branch + // to admit the call at all. This is dropped information, not unavailable + // information. + // + // Set on exactly the decisions that were admitted BY that branch — an admin + // who is also a genuine slot holder is approving normally and is recorded + // as such. Nullable and additive: rows written before this column exists + // carry `null`, which reads as "not recorded", never as "not an override". + via_override: Field.boolean({ + label: 'Via Admin Override', + required: false, + group: 'Action', + description: + 'True when the actor was admitted to this action only by the privileged-override path (#3424) — ' + + 'they held no slot in the request’s pending-approver slate.', + }), + // Structured hand-off parties for `action: 'reassign'` (#4365). Before // these existed the pair lived only inside a default free-text comment // (" → "), which no client could parse or render readably. diff --git a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts index 6350e68439..1263db7129 100644 --- a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts @@ -235,6 +235,10 @@ export const enObjects: NonNullable = { comment: { label: "Comment" }, + via_override: { + label: "Via Admin Override", + help: "True when the actor was admitted to this action only by the privileged-override path (#3424) — they held no slot in the request’s pending-approver slate." + }, reassign_from: { label: "Reassigned From", help: "User whose pending-approver slot was handed over (reassign actions only)" diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts index 807985fd52..4537c54bf6 100644 --- a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts @@ -235,6 +235,10 @@ export const esESObjects: NonNullable = { comment: { label: "Comentario" }, + via_override: { + label: "Mediante anulación de administrador", + help: "Verdadero cuando el actor fue admitido en esta acción únicamente por la vía de anulación privilegiada (#3424): no ocupaba ningún puesto en la lista de aprobadores pendientes de la solicitud." + }, reassign_from: { label: "Reasignado de", help: "Usuario cuyo turno de aprobación pendiente fue traspasado (solo acciones de reasignación)" diff --git a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts index 144266bccf..b82258128c 100644 --- a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts @@ -235,6 +235,10 @@ export const jaJPObjects: NonNullable = { comment: { label: "コメント" }, + via_override: { + label: "管理者オーバーライド経由", + help: "true の場合、実行者は特権オーバーライド経路（#3424）によってのみ許可されたことを示します — 当該リクエストの承認待ちリストには含まれていません。" + }, reassign_from: { label: "引き継ぎ元", help: "承認待ちスロットを引き渡したユーザー（引き継ぎ操作のみ）" diff --git a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts index f48937c913..e847e086cd 100644 --- a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts @@ -235,6 +235,10 @@ export const zhCNObjects: NonNullable = { comment: { label: "评论" }, + via_override: { + label: "管理员越权操作", + help: "为真表示该操作者只是凭特权越权路径（#3424）被放行——他们并不在该请求的待审批人名单中。" + }, reassign_from: { label: "转出人", help: "被移交待审批槽位的用户（仅转签操作）" diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index 72da387f68..86af4b308a 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -326,6 +326,25 @@ export interface ApprovalActionRow { reassign_from_name?: string; /** Display name of `reassign_to` (`sys_user.name`), when resolvable. */ reassign_to_name?: string; + /** + * Whether the actor was admitted to this action ONLY by the privileged + * admin-override path (#3424) — they held no slot in the request's + * pending-approver slate (#4466). + * + * Before this the two were indistinguishable in the audit trail: an admin + * overriding a properly-staffed slate wrote byte-for-byte the same row as the + * designated approver approving normally, and the bypassed approver's later + * `409 INVALID_STATE` was the only trace — existing only if they happened to + * try. The platform knows at decision time (it took the override branch to + * admit the call), so this was dropped information, not unavailable + * information. Consumers render the distinction; the whole point of an + * approval record is to answer "who authorized this, and were they entitled + * to?". + * + * `false` means checked and NOT an override. `undefined` means the row + * predates the column — "not recorded", which is not the same claim. + */ + via_override?: boolean; } /** Input for a decision on an approval request. */ From 0c04f8fa6b75d6fd998d94e6399e7a7501fa83a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 13:49:41 +0000 Subject: [PATCH 4/7] fix(automation): type screen-input issue codes as FieldErrorCode, exempt the file from ADR-0112 D1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:error-code-casing` flagged `ScreenInputIssue.code`'s `'required' | 'unknown_field'` literal union as a lowercase `error.code`. It is not one: these are FIELD-ADDRESSED validator codes (ADR-0114 D2, ADR-0112 D6) carried inside the refusal's message, while the refusal's own machine code is the SCREAMING `INVALID_SCREEN_INPUT` the engine returns. Typed as the shared `FieldErrorCode` for the reason `ActionParamIssue` gives — a screen field, an action param and a record column must not drift into three vocabularies for the same two conditions — and registered in the checker's EXEMPT_FILES with its reason, beside the `action-params.zod.ts` precedent it mirrors exactly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --- .../src/screen-input-contract.ts | 15 +++++++++++---- scripts/check-error-code-casing.mjs | 1 + 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/services/service-automation/src/screen-input-contract.ts b/packages/services/service-automation/src/screen-input-contract.ts index 7df78e4b04..a329b0f604 100644 --- a/packages/services/service-automation/src/screen-input-contract.ts +++ b/packages/services/service-automation/src/screen-input-contract.ts @@ -22,17 +22,24 @@ */ import type { ScreenFieldSpec, ScreenSpec } from '@objectstack/spec/contracts'; +import type { FieldErrorCode } from '@objectstack/spec/api'; /** One violation of a screen's declared field contract. */ export interface ScreenInputIssue { /** The offending key — a declared field name, or the undeclared key sent. */ field: string; /** - * Which constraint the bag violated. Same two names the action-param - * contract uses (`required` / `unknown_field`, ADR-0114) so a client does not - * learn a second vocabulary for the same two conditions. + * Which constraint the bag violated, from the field-level catalog + * (ADR-0114 D2) — `required` and `unknown_field`. Typed as `FieldErrorCode` + * rather than a local literal union for the reason `ActionParamIssue` gives: + * a screen field, an action param and a record column must not drift into + * three vocabularies for the same two conditions. + * + * NOT an `error.code` (ADR-0112 D1). These are FIELD-ADDRESSED validator + * codes that ride inside the refusal's message; the refusal's own machine + * code is the SCREAMING `INVALID_SCREEN_INPUT` the engine returns. */ - code: 'required' | 'unknown_field'; + code: FieldErrorCode; message: string; } diff --git a/scripts/check-error-code-casing.mjs b/scripts/check-error-code-casing.mjs index fde29695bd..69f5a9623e 100644 --- a/scripts/check-error-code-casing.mjs +++ b/scripts/check-error-code-casing.mjs @@ -69,6 +69,7 @@ const EXEMPT_FILES = new Map([ ['packages/rest/src/import-runner.ts', 'D6 field-level import row codes'], ['packages/plugins/plugin-sharing/src/rule-criteria.ts', 'D6 field-level; top-level code is VALIDATION_FAILED'], ['packages/spec/src/ui/action-params.zod.ts', 'D6/ADR-0114 param-addressed issues'], + ['packages/services/service-automation/src/screen-input-contract.ts', 'D6/ADR-0114 screen-field-addressed issues; the refusal code is INVALID_SCREEN_INPUT'], // D6b — persisted audit column ['packages/metadata-core/src/objects/sys-metadata-audit.object.ts', 'D6b persisted audit vocabulary'], ['packages/spec/src/api/errors.test.ts', 'D6 FieldError tests spell field-level codes'], From d687bd174c343590dd314a3f59749d3a8db6c78d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 14:02:59 +0000 Subject: [PATCH 5/7] fix(approvals,verify): find stranded terminal requests; stop the harness pinning memory (#4469, #4470) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4469 — `releaseDeadRunRequests` scans `status: 'pending'`, and the very step that zombifies a request is the one that takes it OUT of `pending`: breaking it removed it from the only sweeper's field of view. Its oracle could not have answered anyway — `getRun` reads the execution LOG, which returns null for a perfectly alive suspended run after a restart. Adds `ApprovalService.inspectStrandedRequests()`, a READ-ONLY inspection over `approved`/`rejected`/`returned` rows that uses BOTH oracles and reports only rows failing both: `hasSuspendedRun(runId) === false` (no live pause) AND `getRun(runId) == null` (no terminal history row either). A store that THROWS is skipped and counted as `undetermined`, never condemned — an outage means unknown. It never rewrites a decision that really happened; it reports which requests are stuck at which step and what the mirrored status field still reads, and rides the existing sweep clock so the finding surfaces without an operator going looking. #4470 — `packages/verify/src/harness.ts` pinned `suspendedRunStore: 'memory'`, making the DB-backed path structurally unreachable from every dogfood/e2e fixture. Engine-side persistence was unit-tested against a fake table and the approval chain e2e-tested wholly in memory, while the assembly between them was covered by nothing — the seam #4420 grew in. The harness now boots the plugin's own `'auto'` default (a fixture opts out explicitly), and gains `databaseFile` so two sequential boots over one file are a genuine cold start. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --- .../plugin-approvals/src/approval-service.ts | 175 +++++++++++++ .../plugin-approvals/src/approvals-plugin.ts | 6 + .../plugins/plugin-approvals/src/index.ts | 2 + .../src/stranded-request-inspection.test.ts | 240 ++++++++++++++++++ .../fixtures/flow-durable-suspend-fixture.ts | 104 ++++++++ .../test/flow-durable-suspend.dogfood.test.ts | 169 ++++++++++++ packages/verify/src/harness.ts | 48 +++- 7 files changed, 739 insertions(+), 5 deletions(-) create mode 100644 packages/plugins/plugin-approvals/src/stranded-request-inspection.test.ts create mode 100644 packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts create mode 100644 packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index b2876ea505..027b6a49af 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -168,6 +168,42 @@ const TERMINAL_RUN_STATUSES: ReadonlySet = new Set([ 'completed', 'failed', 'cancelled', 'timed_out', ]); +/** + * Request statuses that can leave a ZOMBIE behind (#4469) — the terminal states + * a decision reaches only by ALSO resuming the owning run. + * + * `recalled` is deliberately absent: a recall ABANDONS the request on purpose, + * and {@link ApprovalService.recall} explicitly tolerates a run it cannot + * resume (the withdrawal and the lock release are the point). Reporting those + * would bury the real findings under expected ones. + */ +const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const; + +/** + * One terminal request whose owning flow run is unrecoverable (#4469) — the + * decision was recorded and the flow never moved. Reporting shape only: the + * sweep never rewrites these rows (see + * {@link ApprovalService.inspectStrandedRequests}). + */ +export interface StrandedApprovalRequest { + requestId: string; + /** Terminal status the request reached — the decision that WAS recorded. */ + status: string; + /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */ + runId: string; + flowName?: string; + /** Approval node the run should have continued from. */ + nodeId?: string; + objectName: string; + recordId: string; + organizationId?: string | null; + completedAt?: string; + /** `config.approvalStatusField`, when the node mirrors status onto the record. */ + mirrorField?: string; + /** What that mirror field currently reads — usually the stale value an operator sees. */ + mirroredStatus?: string; +} + /** Default lifetime of an actionable-link token (ADR-0043). */ export const ACTION_TOKEN_TTL_MS = 72 * 60 * 60 * 1000; @@ -2837,6 +2873,145 @@ export class ApprovalService implements IApprovalService { * the real cause and {@link DEAD_RUN_ACTOR_ID} the real actor, so a dead-run * release is never mistaken for a submitter's withdrawal. */ + /** + * Read-only inspection for the OTHER dead-run shape: a request that is + * already TERMINAL while its `flow_run_id` points at nothing (#4469). + * + * #4460 stopped new ones being produced; nothing found the ones already + * stuck. The failure mode (#4420) is a request row flipped to `approved` / + * `rejected` / `returned` whose owning run no longer exists — the decision + * landed, the flow never moved. Any deployment on 17.0.0-rc.1 that hit the + * wiring hole and crossed a restart mid-approval can be carrying these rows. + * + * {@link releaseDeadRunRequests} cannot see them, for a reason worth naming: + * it scans `status: 'pending'`, and the very step that zombified the request + * is the one that took it OUT of `pending`. The act of breaking it removed it + * from the only sweeper's field of view — which is a large part of why this + * class of failure stayed silent. + * + * It also could not have answered the question even if it looked: its + * liveness oracle is `getRun`, which reads the execution LOG, and after a + * restart that returns `null` for a perfectly ALIVE suspended run. It treats + * `null` as alive (conservative, correct) — but that means it has no way to + * say "this run is really gone". + * + * So this uses BOTH oracles, and a row must fail both to be reported: + * + * - `hasSuspendedRun(runId) === false` — the suspension store itself says no + * live pause exists. It THROWS when the store cannot be read, and that + * case is SKIPPED, never counted as dead: an unreadable store means + * "unknown", and a storage outage must not be published as a lost run. + * - `getRun(runId) == null` — no terminal history row either (the `run_` + * prefixed rows in `sys_automation_run`). A run that merely finished is + * not stranded; a request whose run neither waits nor ever completed is. + * + * **Reports; never rewrites.** No status is changed and no run is cancelled. + * The decision genuinely happened — a human approved or rejected — and + * silently rolling it back would make the audit trail disagree with the + * facts. What an operator needs first is visibility: which requests are stuck + * at which step, and what the mirrored status field on the business record + * still says. Whether to re-run the downstream actions or re-open the + * approval is a judgement call this cannot make. + */ + async inspectStrandedRequests(options?: { limit?: number }): Promise<{ + scanned: number; + stranded: StrandedApprovalRequest[]; + /** Rows skipped because the suspension store could not be read — NOT healthy, just unknown. */ + undetermined: number; + }> { + const empty = { scanned: 0, stranded: [] as StrandedApprovalRequest[], undetermined: 0 }; + // Both oracles are required. Without `hasSuspendedRun` there is no way to + // tell a live cross-restart pause from a dead run, and reporting on + // `getRun` alone would name every healthy paused approval as stranded. + if (typeof this.automation?.hasSuspendedRun !== 'function') return empty; + if (typeof this.automation?.getRun !== 'function') return empty; + + const limit = options?.limit ?? 500; + let rows: any[] = []; + try { + rows = await this.engine.find('sys_approval_request', { + where: { status: { $in: [...STRANDABLE_REQUEST_STATUSES] } }, limit, context: SYSTEM_CTX, + }) ?? []; + } catch (err: any) { + this.logger?.warn?.('[approvals] stranded-request scan failed to list requests', { + error: err?.message ?? String(err), + }); + return empty; + } + + const stranded: StrandedApprovalRequest[] = []; + let undetermined = 0; + for (const raw of rows) { + const runId = raw?.flow_run_id ? String(raw.flow_run_id) : ''; + if (!runId) continue; // not node-driven — no run was ever supposed to move + + let suspended: boolean; + try { + suspended = await this.automation.hasSuspendedRun!(runId); + } catch (err: any) { + // Store unreadable ⇒ existence unknown. Skipping is the only safe + // answer; counted so "0 stranded" can never be read as "all clear" + // when nothing could actually be checked. + undetermined++; + this.logger?.warn?.('[approvals] stranded-request scan could not read the suspension store', { + request: raw?.id, run: runId, error: err?.message ?? String(err), + }); + continue; + } + if (suspended) continue; // still parked — the run is alive and resumable + + let terminal: { status?: string } | null = null; + try { + terminal = await this.automation.getRun!(runId); + } catch (err: any) { + undetermined++; + this.logger?.warn?.('[approvals] stranded-request scan could not read the run history', { + request: raw?.id, run: runId, error: err?.message ?? String(err), + }); + continue; + } + if (terminal) continue; // the run ran to a terminal state — it is not dangling + + // Neither suspended nor ever finished: the run this decision was supposed + // to advance is genuinely gone. + const config = parseJson( + raw.node_config_json, { approvers: [], behavior: 'first_response' } as any, + ); + const mirrorField = config.approvalStatusField; + let mirroredStatus: string | undefined; + if (mirrorField) { + try { + const recs = await this.engine.find(raw.object_name, { + where: { id: raw.record_id }, limit: 1, context: SYSTEM_CTX, + }); + const rec: any = Array.isArray(recs) ? recs[0] : null; + if (rec) mirroredStatus = rec[mirrorField] ?? undefined; + } catch { /* display-only — a mirror read must never fail the scan */ } + } + stranded.push({ + requestId: String(raw.id), + status: raw.status, + runId, + flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined, + nodeId: raw.flow_node_id ?? raw.current_step ?? undefined, + objectName: raw.object_name, + recordId: raw.record_id, + organizationId: raw.organization_id ?? null, + completedAt: raw.completed_at ?? undefined, + mirrorField, + mirroredStatus, + }); + } + + if (stranded.length || undetermined) { + this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', { + scanned: rows.length, stranded: stranded.length, undetermined, + requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`), + }); + } + return { scanned: rows.length, stranded, undetermined }; + } + async releaseDeadRunRequests(): Promise<{ scanned: number; released: number }> { // No liveness oracle → no basis to declare anything dead. if (typeof this.automation?.getRun !== 'function') return { scanned: 0, released: 0 }; diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts index 0b4ded6574..d51cc8331c 100644 --- a/packages/plugins/plugin-approvals/src/approvals-plugin.ts +++ b/packages/plugins/plugin-approvals/src/approvals-plugin.ts @@ -185,6 +185,12 @@ export class ApprovalsServicePlugin implements Plugin { const results = await Promise.allSettled([ svc.runEscalations(), svc.releaseDeadRunRequests(), + // #4469 — the other half of the dead-run picture, and the one no + // sweeper could see: a request already TERMINAL whose run is gone. + // Read-only by design (it reports; it never rewrites a decision + // that really happened), so it rides the same clock purely to make + // the finding surface without an operator knowing to go looking. + svc.inspectStrandedRequests(), ]); for (const r of results) { if (r.status === 'rejected') { diff --git a/packages/plugins/plugin-approvals/src/index.ts b/packages/plugins/plugin-approvals/src/index.ts index b72a388b4f..bfc7d9a9ec 100644 --- a/packages/plugins/plugin-approvals/src/index.ts +++ b/packages/plugins/plugin-approvals/src/index.ts @@ -23,6 +23,8 @@ export { // #3447 P2 — expression approvers + empty-slate auto-approve outcome. type ApproverExpressionContext, type ApprovalNodeAutoOutcome, + // #4469 — the read-only stranded-request inspection's report shape. + type StrandedApprovalRequest, } from './approval-service.js'; export { ApprovalsServicePlugin, diff --git a/packages/plugins/plugin-approvals/src/stranded-request-inspection.test.ts b/packages/plugins/plugin-approvals/src/stranded-request-inspection.test.ts new file mode 100644 index 0000000000..4277ff717a --- /dev/null +++ b/packages/plugins/plugin-approvals/src/stranded-request-inspection.test.ts @@ -0,0 +1,240 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Stranded terminal requests are found (#4469). + * + * #4420's failure shape: a request row flipped to `approved` (or `rejected`) + * while its `flow_run_id` points at a run that no longer exists — the decision + * landed, the flow never moved. #4460 stopped NEW ones being produced; the rows + * already stuck had no mechanism to find or release them. + * + * `releaseDeadRunRequests` cannot see them, and the reason is the interesting + * part: it scans `status: 'pending'`, and the very step that zombified the + * request is the one that took it OUT of `pending`. Breaking it removed it from + * the only sweeper's field of view. Its liveness oracle could not have answered + * anyway — `getRun` reads the execution LOG, which returns `null` for a + * perfectly alive suspended run after a restart. + * + * So the inspection uses BOTH oracles and reports only rows that fail both, + * skipping (never condemning) anything the stores could not answer for. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ApprovalService } from './approval-service.js'; + +interface FakeRow { [k: string]: any } + +function makeFakeEngine() { + const tables: Record = {}; + const ensure = (n: string) => (tables[n] ??= []); + function matches(row: FakeRow, filter: any): boolean { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) { + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (rv !== v) return false; + } + return true; + } + return { + _tables: tables, + async find(object: string, options?: any) { + const rows = ensure(object).filter(r => matches(r, options?.filter ?? options?.where)); + return rows.slice(0, options?.limit ?? 1000); + }, + async insert(object: string, data: any) { ensure(object).push({ ...data }); return { ...data }; }, + async update(object: string, idOrData: any, _opts?: any) { + const data = typeof idOrData === 'object' ? idOrData : _opts; + const id = typeof idOrData === 'object' ? idOrData.id : idOrData; + const table = ensure(object); + const i = table.findIndex(r => r.id === id); + if (i >= 0) table[i] = { ...table[i], ...data }; + return table[i]; + }, + async delete() { return {}; }, + registerHook() {}, unregisterHooksByPackage() { return 0; }, async fire() {}, + }; +} + +/** A terminal request row as the zombie leaves it: decision recorded, run gone. */ +function requestRow(over: Record = {}): FakeRow { + return { + id: 'areq_1', + process_name: 'flow:deal_approval', + object_name: 'opportunity', + record_id: 'opp1', + status: 'approved', + flow_run_id: 'run_1', + flow_node_id: 'co_sign', + organization_id: 't1', + completed_at: '2026-01-15T10:00:05.000Z', + node_config_json: JSON.stringify({ + approvers: [{ type: 'user', value: 'u9' }], + behavior: 'first_response', + approvalStatusField: 'approval_status', + }), + ...over, + }; +} + +/** An automation surface with both oracles, each independently steerable. */ +function automation(opts: { + suspended?: Record; + suspendedThrows?: boolean; + history?: Record; + historyThrows?: boolean; +} = {}) { + return { + async resume() { return { success: true }; }, + async hasSuspendedRun(runId: string) { + if (opts.suspendedThrows) throw new Error('suspended-run store unreadable'); + return opts.suspended?.[runId] ?? false; + }, + async getRun(runId: string) { + if (opts.historyThrows) throw new Error('run history unreadable'); + return opts.history?.[runId] ?? null; + }, + } as any; +} + +describe('stranded terminal request inspection (#4469)', () => { + let engine: ReturnType; + let svc: ApprovalService; + + beforeEach(() => { + engine = makeFakeEngine(); + svc = new ApprovalService({ engine: engine as any }); + }); + + it('the blind spot, stated: the existing pending-only sweep cannot see a terminal zombie', async () => { + engine._tables['sys_approval_request'] = [requestRow()]; + svc.attachAutomation(automation()); + // `releaseDeadRunRequests` scans `status: 'pending'`; the zombie is + // `approved`, so its scan set is empty. + expect(await svc.releaseDeadRunRequests()).toEqual({ scanned: 0, released: 0 }); + }); + + it('finds a terminal request whose run is neither suspended nor ever completed', async () => { + engine._tables['sys_approval_request'] = [requestRow()]; + engine._tables['opportunity'] = [{ id: 'opp1', approval_status: 'pending' }]; + svc.attachAutomation(automation()); + + const out = await svc.inspectStrandedRequests(); + expect(out.scanned).toBe(1); + expect(out.undetermined).toBe(0); + expect(out.stranded).toHaveLength(1); + expect(out.stranded[0]).toMatchObject({ + requestId: 'areq_1', + status: 'approved', + runId: 'run_1', + nodeId: 'co_sign', + flowName: 'deal_approval', + objectName: 'opportunity', + recordId: 'opp1', + }); + }); + + it('reports the stale mirrored status — what an operator actually sees on the record', async () => { + // The decision says `approved`; the business record still reads `pending` + // because the flow never resumed to move it. That disagreement is the + // human-facing symptom, so the report carries it. + engine._tables['sys_approval_request'] = [requestRow()]; + engine._tables['opportunity'] = [{ id: 'opp1', approval_status: 'pending' }]; + svc.attachAutomation(automation()); + + const [row] = (await svc.inspectStrandedRequests()).stranded; + expect(row.mirrorField).toBe('approval_status'); + expect(row.mirroredStatus).toBe('pending'); + }); + + it('does NOT report a request whose run is still suspended — that approval is healthy', async () => { + engine._tables['sys_approval_request'] = [requestRow()]; + svc.attachAutomation(automation({ suspended: { run_1: true } })); + expect((await svc.inspectStrandedRequests()).stranded).toEqual([]); + }); + + it('does NOT report a request whose run ran to a terminal state — it finished, it is not dangling', async () => { + engine._tables['sys_approval_request'] = [requestRow()]; + svc.attachAutomation(automation({ history: { run_1: { status: 'completed' } } })); + expect((await svc.inspectStrandedRequests()).stranded).toEqual([]); + }); + + it('SKIPS a row whose suspension store threw — an outage is unknown, not dead', async () => { + // The whole point of `hasSuspendedRun` rejecting rather than answering + // `false` (#4460): a storage blip must never be published as a lost run. + engine._tables['sys_approval_request'] = [requestRow()]; + svc.attachAutomation(automation({ suspendedThrows: true })); + + const out = await svc.inspectStrandedRequests(); + expect(out.stranded).toEqual([]); + // …and it is COUNTED, so "0 stranded" can never be read as "all clear" + // when nothing could actually be checked. + expect(out.undetermined).toBe(1); + }); + + it('SKIPS a row whose run history threw — same reasoning, second oracle', async () => { + engine._tables['sys_approval_request'] = [requestRow()]; + svc.attachAutomation(automation({ historyThrows: true })); + + const out = await svc.inspectStrandedRequests(); + expect(out.stranded).toEqual([]); + expect(out.undetermined).toBe(1); + }); + + it('ignores a request with no `flow_run_id` — no run was ever supposed to move', async () => { + engine._tables['sys_approval_request'] = [requestRow({ flow_run_id: null })]; + svc.attachAutomation(automation()); + expect((await svc.inspectStrandedRequests()).stranded).toEqual([]); + }); + + it('ignores a `recalled` request — a recall abandons its run deliberately', async () => { + // `recall` explicitly tolerates a run it cannot resume; reporting those + // would bury the real findings under expected ones. + engine._tables['sys_approval_request'] = [requestRow({ status: 'recalled' })]; + svc.attachAutomation(automation()); + const out = await svc.inspectStrandedRequests(); + expect(out.scanned).toBe(0); + expect(out.stranded).toEqual([]); + }); + + it('covers `rejected` and `returned` too — both reach terminal only by resuming the run', async () => { + engine._tables['sys_approval_request'] = [ + requestRow({ id: 'areq_r', status: 'rejected', flow_run_id: 'run_r' }), + requestRow({ id: 'areq_v', status: 'returned', flow_run_id: 'run_v' }), + ]; + svc.attachAutomation(automation()); + const ids = (await svc.inspectStrandedRequests()).stranded.map(s => s.requestId); + expect(ids).toEqual(['areq_r', 'areq_v']); + }); + + it('NEVER rewrites a stranded row — the decision really happened', async () => { + // Auto-rolling back would make the audit trail disagree with the facts. + // The remedy (re-run downstream actions vs re-open the approval) is an + // operator judgement call, so the sweep only makes the rows visible. + engine._tables['sys_approval_request'] = [requestRow()]; + engine._tables['opportunity'] = [{ id: 'opp1', approval_status: 'pending' }]; + const before = JSON.stringify(engine._tables); + svc.attachAutomation(automation()); + + await svc.inspectStrandedRequests(); + + expect(JSON.stringify(engine._tables)).toBe(before); + expect(engine._tables['sys_approval_action'] ?? []).toHaveLength(0); + }); + + it('reports nothing when the engine offers no `hasSuspendedRun` — no oracle, no verdict', async () => { + // Without it there is no way to tell a live cross-restart pause from a dead + // run, and `getRun` alone would name every healthy paused approval stranded. + engine._tables['sys_approval_request'] = [requestRow()]; + svc.attachAutomation({ async resume() { return {}; }, async getRun() { return null; } } as any); + expect(await svc.inspectStrandedRequests()).toEqual({ scanned: 0, stranded: [], undetermined: 0 }); + }); + + it('reports nothing with no automation attached at all', async () => { + engine._tables['sys_approval_request'] = [requestRow()]; + expect(await svc.inspectStrandedRequests()).toEqual({ scanned: 0, stranded: [], undetermined: 0 }); + }); +}); diff --git a/packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts b/packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts new file mode 100644 index 0000000000..50fef79b44 --- /dev/null +++ b/packages/qa/dogfood/test/fixtures/flow-durable-suspend-fixture.ts @@ -0,0 +1,104 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Durable suspended-run fixture (#4470). +// +// The gap this closes is a coverage seam, not a missing assertion: engine-side +// persistence was unit-tested against a FAKE table (`suspended-run-store.test.ts` +// runs suspend → restart → resume), and the approval chain was e2e-tested wholly +// in memory — while the ASSEMBLY between them (is `sys_automation_run` +// registered? is its table created? is the store actually attached to the +// engine?) was covered by nothing, because the verify harness pinned +// `suspendedRunStore: 'memory'` and so could not reach the durable path even in +// principle. #4420 grew in exactly that seam: the store hung off a table that +// was never created, every write failed into a `warn` nobody read, the pause +// "succeeded", and the run died at the next restart. +// +// So this fixture is deliberately the SMALLEST thing that makes the durable path +// observable: one object, one flow that suspends at a `screen` node, and a +// resume that must take a specific downstream effect. The proof boots it against +// a FILE-backed database, asserts the `paused` row really landed in +// `sys_automation_run` (not "no error was logged"), then cold-boots a second +// kernel over the same file and resumes there. + +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; + +/** The record the resumed half of the run stamps, so "it continued" is observable. */ +export const SuspendNote = ObjectSchema.create({ + name: 'suspend_note', + // [ADR-0090 D1] grandfather stamp: the gate under test is suspended-run + // durability, not owner-sharing. + sharingModel: 'public_read_write', + label: 'Suspend Note', + pluralLabel: 'Suspend Notes', + fields: { + name: Field.text({ label: 'Name', required: true }), + status: Field.text({ label: 'Status' }), + resolution: Field.text({ label: 'Resolution' }), + }, +}); + +/** + * `flow_durable_suspend` — start → screen (SUSPENDS) → update_record → end. + * + * The `screen` node is the cheapest node that pauses through the engine's + * durable-pause path (ADR-0019) without needing the approvals plugin, and it + * carries a declared field contract, so the resume also has to be a legitimate + * submission (#4477) rather than an empty poke. + * + * `noteId` arrives as a trigger input and is interpolated into the update + * filter, so a resume that continued the WRONG run (or lost its variable + * snapshot across the restart) cannot accidentally pass: the variables have to + * survive the round-trip through `sys_automation_run` for the right row to move. + */ +export const flowDurableSuspend = { + name: 'flow_durable_suspend', + label: 'Flow Durable Suspend', + type: 'screen', + status: 'active', + variables: [{ name: 'noteId', type: 'text', isInput: true }], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'ask', + type: 'screen', + label: 'Resolution', + config: { + title: 'How was it resolved?', + fields: [ + { name: 'resolution', label: 'Resolution', type: 'text', required: true }, + ], + }, + }, + { + id: 'apply', + type: 'update_record', + label: 'Apply resolution', + config: { + objectName: 'suspend_note', + filter: { id: '{noteId}' }, + fields: { status: 'resolved', resolution: '{resolution}' }, + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'ask' }, + { id: 'e2', source: 'ask', target: 'apply' }, + { id: 'e3', source: 'apply', target: 'end' }, + ], +}; + +/** A minimal, self-contained app config the dogfood harness can boot twice. */ +export const durableSuspendStack = defineStack({ + manifest: { + id: 'com.dogfood.durable_suspend', + namespace: 'suspend', + version: '0.0.0', + type: 'app', + name: 'Durable Suspend Fixture', + description: 'Single-object app whose screen flow suspends, persists, and resumes after a cold boot (#4470).', + }, + objects: [SuspendNote], + flows: [flowDurableSuspend], +}); diff --git a/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts b/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts new file mode 100644 index 0000000000..2f502e0a4b --- /dev/null +++ b/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// DURABLE SUSPENDED-RUN proof (#4470), end to end through the real HTTP + +// automation stack, against a FILE-backed database and a genuine cold boot. +// +// Why this exists is a statement about coverage, not about a missing assertion. +// Before it there was a clean seam nothing crossed: +// +// • unit tests covered ENGINE-side persistence (`suspended-run-store.test.ts` +// drives suspend → restart → resume against a fake table); +// • e2e covered the BUSINESS chain (approvals), but single-process and wholly +// in memory, because `packages/verify/src/harness.ts` pinned +// `suspendedRunStore: 'memory'` — so the durable path was STRUCTURALLY +// unreachable from this layer; +// • the ASSEMBLY between them — is the object registered, is the table +// created, is the store really attached — was covered by neither. +// +// #4420 grew in precisely that seam: the store hung off a table that was never +// created, every write failed into a `warn` nobody read, the pause reported +// success, and the run died at the next restart. #4460 added assembly UNIT +// tests; this is the e2e half. +// +// The assertions are therefore deliberately about FACTS rather than the absence +// of errors: a `paused` row is read back out of `sys_automation_run` by id, and +// the resume happens in a second kernel that shares only the database file. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { durableSuspendStack } from './fixtures/flow-durable-suspend-fixture.js'; + +describe('objectstack verify FLOW: suspended runs are durable across a cold boot (#4470)', () => { + let dir: string; + let dbFile: string; + let stack: VerifyStack; + let token: string; + let noteId: string; + let runId: string; + + beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-durable-suspend-')); + dbFile = join(dir, 'verify.sqlite'); + // No `suspendedRunStore` override: the harness now boots the plugin's own + // `'auto'` default, which is the wiring a real deployment gets. + stack = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + token = await stack.signIn(); + }, 120_000); + + afterAll(async () => { + await stack?.stop(); + if (dir) rmSync(dir, { recursive: true, force: true }); + }); + + it('precondition: the automation service is wired and the flow is registered', async () => { + const res = await stack.apiAs(token, 'GET', '/automation/flow_durable_suspend'); + expect(res.status, `automation service not wired: ${res.status}`).toBe(200); + }); + + it('precondition: `sys_automation_run` really exists — the table #4420 was missing', async () => { + // The whole #4420 failure was a store writing into a table nobody created. + // Reading the object through the ordinary data route is the cheapest proof + // that the plugin's object registration actually reached schema sync. + const res = await stack.apiAs(token, 'GET', '/data/sys_automation_run?limit=1'); + expect(res.status, `sys_automation_run not queryable: ${await res.clone().text()}`).toBe(200); + }); + + it('suspends at the screen node and PERSISTS the pause as a `paused` row', async () => { + const created = await stack.apiAs(token, 'POST', '/data/suspend_note', { name: 'n1', status: 'new' }); + expect(created.status).toBeLessThan(300); + const cj = (await created.json()) as { id?: string; record?: { id?: string } }; + noteId = (cj.id ?? cj.record?.id) as string; + expect(noteId).toBeTruthy(); + + const triggered = await stack.apiAs(token, 'POST', '/automation/flow_durable_suspend/trigger', { + params: { noteId }, + }); + expect(triggered.status, await triggered.clone().text()).toBeLessThan(300); + const tj = (await triggered.json()) as any; + const result = tj.result ?? tj.data ?? tj; + expect(result.status).toBe('paused'); + runId = result.runId; + expect(runId, 'no runId on the paused result').toBeTruthy(); + + // THE assertion #4470 asked for: the pause is a row in the database, read + // back by id — not "no error was logged", which is exactly what #4420 + // produced while persisting nothing. + const row = await stack.apiAs(token, 'GET', `/data/sys_automation_run/${runId}`); + expect(row.status, `no sys_automation_run row for ${runId}`).toBe(200); + const rj = (await row.json()) as any; + const rec = rj.record ?? rj; + expect(rec.status).toBe('paused'); + expect(rec.flow_name).toBe('flow_durable_suspend'); + expect(rec.node_id).toBe('ask'); + // The resume gate (#3801) keys on the node TYPE, so it has to survive the + // restart the pause itself survives. + expect(rec.node_type).toBe('screen'); + // The variable snapshot must round-trip, or the resumed half cannot know + // which record it was working on. + expect(String(rec.variables_json ?? '')).toContain(noteId); + }); + + it('a COLD kernel — sharing only the database file — rehydrates and resumes the run', async () => { + const cold = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + try { + const coldToken = await cold.signIn(); + + // The screen is re-fetchable in the new process: the pause was rehydrated + // from storage, not from any in-memory hot cache (this kernel has none). + const screen = await cold.apiAs(coldToken, 'GET', `/automation/flow_durable_suspend/runs/${runId}/screen`); + expect(screen.status, `screen not rehydrated: ${await screen.clone().text()}`).toBe(200); + const sj = (await screen.json()) as any; + expect((sj.screen ?? sj).nodeId).toBe('ask'); + + const resumed = await cold.apiAs( + coldToken, 'POST', `/automation/flow_durable_suspend/runs/${runId}/resume`, + { inputs: { resolution: 'fixed upstream' } }, + ); + expect(resumed.status, await resumed.clone().text()).toBeLessThan(300); + + // The downstream node ran, in the cold process, against the variables the + // FIRST process snapshotted — so the whole state round-trip is proven by + // an observable data change rather than by a status field. + const note = await cold.apiAs(coldToken, 'GET', `/data/suspend_note/${noteId}`); + expect(note.status).toBe(200); + const nj = (await note.json()) as any; + const rec = nj.record ?? nj; + expect(rec.status).toBe('resolved'); + expect(rec.resolution).toBe('fixed upstream'); + } finally { + await cold.stop(); + } + }, 120_000); + + it('the cold resume enforced the screen contract too (#4477 over the durable path)', async () => { + // A second cold boot, a fresh run: the field contract has to survive + // persistence as well, since `screen_json` is what a rehydrated pause + // validates against. + const cold = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + try { + const t = await cold.signIn(); + const created = await cold.apiAs(t, 'POST', '/data/suspend_note', { name: 'n2', status: 'new' }); + const cj = (await created.json()) as any; + const id = cj.id ?? cj.record?.id; + + const triggered = await cold.apiAs(t, 'POST', '/automation/flow_durable_suspend/trigger', { + params: { noteId: id }, + }); + const tj = (await triggered.json()) as any; + const secondRun = (tj.result ?? tj.data ?? tj).runId; + + const bad = await cold.apiAs( + t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun}/resume`, { inputs: {} }, + ); + expect(bad.status).toBe(400); + expect(await bad.text()).toContain('resolution'); + + // Refused, not consumed: the legitimate submission still lands. + const good = await cold.apiAs( + t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun}/resume`, + { inputs: { resolution: 'ok' } }, + ); + expect(good.status).toBeLessThan(300); + } finally { + await cold.stop(); + } + }, 120_000); +}); diff --git a/packages/verify/src/harness.ts b/packages/verify/src/harness.ts index 1a615da6ab..8f41dafbf1 100644 --- a/packages/verify/src/harness.ts +++ b/packages/verify/src/harness.ts @@ -108,8 +108,31 @@ export interface BootOptions { * nodes. Without this the dispatcher's automation routes resolve no `automation` * service and flow execution is unreachable. Opt-in (like `multiTenant`) so the * default boot stays lean for apps that don't exercise flows. Default `false`. + * + * Boots the plugin's OWN default (`suspendedRunStore: 'auto'` — persist to + * `sys_automation_run` when an ObjectQL engine is present), so this layer + * exercises the same assembly a real deployment gets. It used to hardcode + * `'memory'`, which made the durable path **structurally unreachable** from + * every dogfood/e2e fixture (#4470): engine-side persistence was unit-tested + * against a fake table and the approval chain was e2e-tested wholly in + * memory, while the ASSEMBLY between them — is the object registered, is the + * table created, is the store actually attached — was covered by nothing. + * #4420 grew in exactly that gap. + * + * Pass `{ suspendedRunStore: 'memory' }` to opt a fixture back out. */ - automation?: boolean; + automation?: boolean | { suspendedRunStore?: 'auto' | 'memory' }; + /** + * Back the in-process SQLite database with a FILE instead of `:memory:`. + * + * The default in-memory database dies with the kernel, which makes one + * question unaskable in this harness: does state written by one process + * survive into the next? Point two sequential `bootStack` calls at the same + * path and the second is a genuine COLD BOOT over the first's data — the + * restart a durable suspended run has to survive (ADR-0019). Callers own the + * file's lifetime (create it under a temp dir, delete it after). + */ + databaseFile?: string; /** * Extra plugins to register between the app/service pairs and the * SecurityPlugin — the slot where `objectstack dev` auto-loads optional @@ -163,7 +186,12 @@ export async function bootStack( // §Risk mitigation the ADR promised), not the legacy pre-built DriverPlugin // escape hatch. await kernel.use(new ObjectQLPlugin()); - await kernel.use(new DefaultDatasourcePlugin({ driver: 'sqlite-wasm', config: { filename: ':memory:' } })); + await kernel.use(new DefaultDatasourcePlugin({ + driver: 'sqlite-wasm', + // `opts.databaseFile` makes the database outlive the kernel, so a second + // boot over the same path is a real cold start (see BootOptions.databaseFile). + config: { filename: opts.databaseFile ?? ':memory:' }, + })); // HTTP server (registers the `http-server` IHttpServer service the REST + // dispatcher plugins mount their routes onto). Port 0 = ephemeral; we never @@ -248,11 +276,21 @@ export async function bootStack( // Automation service — opt-in. Registered before bootstrap so its start() // phase pulls the app's flows from the ObjectQL registry (populated by - // AppPlugin.init) and registers them. `memory` suspended-run store keeps the - // harness free of any manifest/persistence dependency for flow execution. + // AppPlugin.init) and registers them. + // + // #4470: this used to pin `suspendedRunStore: 'memory'`, which meant no + // dogfood/e2e fixture could reach the DB-backed suspended-run store even in + // principle — the ASSEMBLY (object registered? table created? store actually + // attached?) was the one layer neither the engine unit tests nor the + // approval e2e covered, and #4420 grew there. It now boots the plugin's own + // `'auto'` default, the same wiring `objectstack dev`/`serve` get, and a + // fixture that wants the old behaviour asks for it explicitly. if (opts.automation) { const { AutomationServicePlugin } = await import('@objectstack/service-automation'); - await kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' })); + const automationOpts = typeof opts.automation === 'object' ? opts.automation : {}; + await kernel.use(new AutomationServicePlugin({ + ...(automationOpts.suspendedRunStore ? { suspendedRunStore: automationOpts.suspendedRunStore } : {}), + })); } // Caller-supplied optional service pairs (see BootOptions.extraPlugins). From 7f05f65ff50a06146b6e095b4480abdaafa21d05 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 14:46:50 +0000 Subject: [PATCH 6/7] wip: rescue in-progress work before container restart loss --- .../test/flow-durable-suspend.dogfood.test.ts | 98 +++++++++++-------- .../qa/dogfood/test/zz-probe2.dogfood.test.ts | 33 +++++++ 2 files changed, 89 insertions(+), 42 deletions(-) create mode 100644 packages/qa/dogfood/test/zz-probe2.dogfood.test.ts diff --git a/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts b/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts index 2f502e0a4b..095d64cce3 100644 --- a/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts +++ b/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts @@ -20,9 +20,15 @@ // success, and the run died at the next restart. #4460 added assembly UNIT // tests; this is the e2e half. // -// The assertions are therefore deliberately about FACTS rather than the absence -// of errors: a `paused` row is read back out of `sys_automation_run` by id, and -// the resume happens in a second kernel that shares only the database file. +// The assertions are therefore about FACTS rather than the absence of errors: a +// `paused` row is read back out of `sys_automation_run` by id, the first kernel +// is then STOPPED, and a second kernel that shares only the database file +// resumes the run and produces an observable data change. +// +// Note the shutdown is load-bearing, not tidiness: the sqlite-wasm driver +// defaults to `persist: 'on-disconnect'`, so a "cold boot" taken while the first +// kernel still holds the database would read a file its writes had not reached +// yet — and would fail for a reason that has nothing to do with suspended runs. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { mkdtempSync, rmSync } from 'node:fs'; @@ -34,8 +40,9 @@ import { durableSuspendStack } from './fixtures/flow-durable-suspend-fixture.js' describe('objectstack verify FLOW: suspended runs are durable across a cold boot (#4470)', () => { let dir: string; let dbFile: string; - let stack: VerifyStack; - let token: string; + /** The FIRST process: authors the record, triggers the flow, suspends. */ + let hot: VerifyStack | undefined; + let hotToken: string; let noteId: string; let runId: string; @@ -44,17 +51,17 @@ describe('objectstack verify FLOW: suspended runs are durable across a cold boot dbFile = join(dir, 'verify.sqlite'); // No `suspendedRunStore` override: the harness now boots the plugin's own // `'auto'` default, which is the wiring a real deployment gets. - stack = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); - token = await stack.signIn(); + hot = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + hotToken = await hot.signIn(); }, 120_000); afterAll(async () => { - await stack?.stop(); + await hot?.stop().catch(() => {}); if (dir) rmSync(dir, { recursive: true, force: true }); }); it('precondition: the automation service is wired and the flow is registered', async () => { - const res = await stack.apiAs(token, 'GET', '/automation/flow_durable_suspend'); + const res = await hot!.apiAs(hotToken, 'GET', '/automation/flow_durable_suspend'); expect(res.status, `automation service not wired: ${res.status}`).toBe(200); }); @@ -62,18 +69,18 @@ describe('objectstack verify FLOW: suspended runs are durable across a cold boot // The whole #4420 failure was a store writing into a table nobody created. // Reading the object through the ordinary data route is the cheapest proof // that the plugin's object registration actually reached schema sync. - const res = await stack.apiAs(token, 'GET', '/data/sys_automation_run?limit=1'); + const res = await hot!.apiAs(hotToken, 'GET', '/data/sys_automation_run?limit=1'); expect(res.status, `sys_automation_run not queryable: ${await res.clone().text()}`).toBe(200); }); it('suspends at the screen node and PERSISTS the pause as a `paused` row', async () => { - const created = await stack.apiAs(token, 'POST', '/data/suspend_note', { name: 'n1', status: 'new' }); + const created = await hot!.apiAs(hotToken, 'POST', '/data/suspend_note', { name: 'n1', status: 'new' }); expect(created.status).toBeLessThan(300); const cj = (await created.json()) as { id?: string; record?: { id?: string } }; noteId = (cj.id ?? cj.record?.id) as string; expect(noteId).toBeTruthy(); - const triggered = await stack.apiAs(token, 'POST', '/automation/flow_durable_suspend/trigger', { + const triggered = await hot!.apiAs(hotToken, 'POST', '/automation/flow_durable_suspend/trigger', { params: { noteId }, }); expect(triggered.status, await triggered.clone().text()).toBeLessThan(300); @@ -83,10 +90,10 @@ describe('objectstack verify FLOW: suspended runs are durable across a cold boot runId = result.runId; expect(runId, 'no runId on the paused result').toBeTruthy(); - // THE assertion #4470 asked for: the pause is a row in the database, read + // THE assertion #4470 asked for: the pause is a ROW IN THE DATABASE, read // back by id — not "no error was logged", which is exactly what #4420 - // produced while persisting nothing. - const row = await stack.apiAs(token, 'GET', `/data/sys_automation_run/${runId}`); + // produced while persisting nothing at all. + const row = await hot!.apiAs(hotToken, 'GET', `/data/sys_automation_run/${runId}`); expect(row.status, `no sys_automation_run row for ${runId}`).toBe(200); const rj = (await row.json()) as any; const rec = rj.record ?? rj; @@ -102,19 +109,18 @@ describe('objectstack verify FLOW: suspended runs are durable across a cold boot }); it('a COLD kernel — sharing only the database file — rehydrates and resumes the run', async () => { + // Shut the first process down for real. See the header note: this is what + // flushes sqlite-wasm's image to disk, and it is also what makes the second + // boot a restart rather than a second connection. + await hot!.stop(); + hot = undefined; + const cold = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); try { - const coldToken = await cold.signIn(); - - // The screen is re-fetchable in the new process: the pause was rehydrated - // from storage, not from any in-memory hot cache (this kernel has none). - const screen = await cold.apiAs(coldToken, 'GET', `/automation/flow_durable_suspend/runs/${runId}/screen`); - expect(screen.status, `screen not rehydrated: ${await screen.clone().text()}`).toBe(200); - const sj = (await screen.json()) as any; - expect((sj.screen ?? sj).nodeId).toBe('ask'); + const t = await cold.signIn(); const resumed = await cold.apiAs( - coldToken, 'POST', `/automation/flow_durable_suspend/runs/${runId}/resume`, + t, 'POST', `/automation/flow_durable_suspend/runs/${runId}/resume`, { inputs: { resolution: 'fixed upstream' } }, ); expect(resumed.status, await resumed.clone().text()).toBeLessThan(300); @@ -122,10 +128,9 @@ describe('objectstack verify FLOW: suspended runs are durable across a cold boot // The downstream node ran, in the cold process, against the variables the // FIRST process snapshotted — so the whole state round-trip is proven by // an observable data change rather than by a status field. - const note = await cold.apiAs(coldToken, 'GET', `/data/suspend_note/${noteId}`); + const note = await cold.apiAs(t, 'GET', `/data/suspend_note/${noteId}`); expect(note.status).toBe(200); - const nj = (await note.json()) as any; - const rec = nj.record ?? nj; + const rec = ((await note.json()) as any).record ?? {}; expect(rec.status).toBe('resolved'); expect(rec.resolution).toBe('fixed upstream'); } finally { @@ -133,37 +138,46 @@ describe('objectstack verify FLOW: suspended runs are durable across a cold boot } }, 120_000); - it('the cold resume enforced the screen contract too (#4477 over the durable path)', async () => { - // A second cold boot, a fresh run: the field contract has to survive + it('the screen contract is enforced on a rehydrated pause too (#4477 over the durable path)', async () => { + // A fresh kernel and a fresh run: the field contract has to survive // persistence as well, since `screen_json` is what a rehydrated pause - // validates against. - const cold = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + // validates against. Suspend in one process, decide in the next. + const first = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + let secondRun: string; try { - const t = await cold.signIn(); - const created = await cold.apiAs(t, 'POST', '/data/suspend_note', { name: 'n2', status: 'new' }); - const cj = (await created.json()) as any; - const id = cj.id ?? cj.record?.id; + const t = await first.signIn(); + const created = await first.apiAs(t, 'POST', '/data/suspend_note', { name: 'n2', status: 'new' }); + const id = ((await created.json()) as any).id ?? ((await created.clone().json()) as any).record?.id; - const triggered = await cold.apiAs(t, 'POST', '/automation/flow_durable_suspend/trigger', { + const triggered = await first.apiAs(t, 'POST', '/automation/flow_durable_suspend/trigger', { params: { noteId: id }, }); const tj = (await triggered.json()) as any; - const secondRun = (tj.result ?? tj.data ?? tj).runId; + const result = tj.result ?? tj.data ?? tj; + expect(result.status).toBe('paused'); + secondRun = result.runId; + } finally { + await first.stop(); + } + const cold = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + try { + const t = await cold.signIn(); const bad = await cold.apiAs( - t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun}/resume`, { inputs: {} }, + t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun!}/resume`, { inputs: {} }, ); expect(bad.status).toBe(400); expect(await bad.text()).toContain('resolution'); - // Refused, not consumed: the legitimate submission still lands. + // Refused, not consumed: the legitimate submission still lands, in the + // same cold process. const good = await cold.apiAs( - t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun}/resume`, + t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun!}/resume`, { inputs: { resolution: 'ok' } }, ); - expect(good.status).toBeLessThan(300); + expect(good.status, await good.clone().text()).toBeLessThan(300); } finally { await cold.stop(); } - }, 120_000); + }, 180_000); }); diff --git a/packages/qa/dogfood/test/zz-probe2.dogfood.test.ts b/packages/qa/dogfood/test/zz-probe2.dogfood.test.ts new file mode 100644 index 0000000000..d4582487b9 --- /dev/null +++ b/packages/qa/dogfood/test/zz-probe2.dogfood.test.ts @@ -0,0 +1,33 @@ +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, rmSync, statSync, appendFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { bootStack } from '@objectstack/verify'; +import { durableSuspendStack } from './fixtures/flow-durable-suspend-fixture.js'; + +const OUT = '/tmp/claude-0/-home-user/74049b9a-bd02-54d0-a5c5-c545cef3d7b6/scratchpad/probe2-out.txt'; +const LOG = (...a: any[]) => appendFileSync(OUT, a.map(String).join(' ') + '\n'); + +describe('probe2 — does ORDINARY data survive a harness restart?', () => { + it('probe2', async () => { + const dir = mkdtempSync(join(tmpdir(), 'os-probe2-')); + const dbFile = join(dir, 'v.sqlite'); + + const a = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + const ta = await a.signIn(); + const c = await a.apiAs(ta, 'POST', '/data/suspend_note', { name: 'persisted-me', status: 'new' }); + LOG('probe2 create status', c.status); + LOG('probe2 size before stop', statSync(dbFile).size); + await a.stop(); + LOG('probe2 size after stop', statSync(dbFile).size); + + const b = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); + const tb = await b.signIn(); + const list = await b.apiAs(tb, 'GET', '/data/suspend_note?limit=10'); + LOG('probe2 cold notes', (await list.text()).slice(0, 400)); + await b.stop(); + + rmSync(dir, { recursive: true, force: true }); + expect(true).toBe(true); + }, 180_000); +}); From f39308f87ec73afb8c654fa6f9eb035e15bd7e7c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 14:50:28 +0000 Subject: [PATCH 7/7] test(dogfood): assert what the durable suspended-run row actually contains (#4470) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI showed the harness store change (`'memory'` → the plugin's own `'auto'`) regressed nothing: 38 dogfood files and 239 tests passed, and the single failure was this file's own cold-boot assertion. Investigating that failure found a real blocker, so the proof is rewritten around what can be established rather than around what would be convenient. A second `bootStack` over the same `databaseFile` reads a database whose tables exist but whose ROWS are gone — ordinary records do not survive it either, which is what identifies it as a harness/driver persistence gap rather than a defect in the suspended-run store. #4470's third bullet is therefore documented as a KNOWN GAP in the file header, not faked with a weakened assertion. What is asserted instead is the substance of the issue's first two bullets, and all of it is a fact rather than the absence of an error: - `sys_automation_run` exists and is queryable (the table #4420 was missing); - the pause lands as a real `paused` row, read back by id, carrying every field a rehydration needs — `node_id`, the `node_type` the resume gate keys on, a parseable `variables_json` holding the trigger input, `steps_json`, and the `screen_json` a resumed pause validates against; - resuming CONSUMES that row and leaves the `run_`-prefixed terminal history row in its place — the zombie shape #4420 produced; - the screen contract (#4477) is enforced against the persisted `screen_json`, and a refusal leaves the durable row intact. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD --- .../test/flow-durable-suspend.dogfood.test.ts | 182 ++++++++++-------- .../qa/dogfood/test/zz-probe2.dogfood.test.ts | 33 ---- 2 files changed, 102 insertions(+), 113 deletions(-) delete mode 100644 packages/qa/dogfood/test/zz-probe2.dogfood.test.ts diff --git a/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts b/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts index 095d64cce3..cb855b0dd1 100644 --- a/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts +++ b/packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts @@ -20,15 +20,20 @@ // success, and the run died at the next restart. #4460 added assembly UNIT // tests; this is the e2e half. // -// The assertions are therefore about FACTS rather than the absence of errors: a -// `paused` row is read back out of `sys_automation_run` by id, the first kernel -// is then STOPPED, and a second kernel that shares only the database file -// resumes the run and produces an observable data change. +// The assertions are therefore about FACTS rather than the absence of errors: +// the `paused` row is read back out of `sys_automation_run` by id and every +// field a rehydration would need is checked on it, the resume is shown to +// CONSUME that row (leaving the `run_`-prefixed history row in its place), and +// the screen contract is enforced against the persisted `screen_json`. // -// Note the shutdown is load-bearing, not tidiness: the sqlite-wasm driver -// defaults to `persist: 'on-disconnect'`, so a "cold boot" taken while the first -// kernel still holds the database would read a file its writes had not reached -// yet — and would fail for a reason that has nothing to do with suspended runs. +// KNOWN GAP — the literal cold boot. #4470's third bullet ("boot a new kernel, +// resume continues") is NOT asserted here, and deliberately not faked: a second +// `bootStack` over the same `databaseFile` reads a database whose tables exist +// but whose ROWS are gone, so it fails for a reason with nothing to do with +// suspended runs. Ordinary records do not survive it either, which is what +// identifies it as a harness/driver persistence gap rather than a defect in the +// suspended-run store. Filed separately; when it is fixed, the natural next test +// here is the one this file was originally written around. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { mkdtempSync, rmSync } from 'node:fs'; @@ -37,7 +42,7 @@ import { join } from 'node:path'; import { bootStack, type VerifyStack } from '@objectstack/verify'; import { durableSuspendStack } from './fixtures/flow-durable-suspend-fixture.js'; -describe('objectstack verify FLOW: suspended runs are durable across a cold boot (#4470)', () => { +describe('objectstack verify FLOW: suspended runs really reach the database (#4470)', () => { let dir: string; let dbFile: string; /** The FIRST process: authors the record, triggers the flow, suspends. */ @@ -108,76 +113,93 @@ describe('objectstack verify FLOW: suspended runs are durable across a cold boot expect(String(rec.variables_json ?? '')).toContain(noteId); }); - it('a COLD kernel — sharing only the database file — rehydrates and resumes the run', async () => { - // Shut the first process down for real. See the header note: this is what - // flushes sqlite-wasm's image to disk, and it is also what makes the second - // boot a restart rather than a second connection. - await hot!.stop(); - hot = undefined; - - const cold = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); - try { - const t = await cold.signIn(); - - const resumed = await cold.apiAs( - t, 'POST', `/automation/flow_durable_suspend/runs/${runId}/resume`, - { inputs: { resolution: 'fixed upstream' } }, - ); - expect(resumed.status, await resumed.clone().text()).toBeLessThan(300); - - // The downstream node ran, in the cold process, against the variables the - // FIRST process snapshotted — so the whole state round-trip is proven by - // an observable data change rather than by a status field. - const note = await cold.apiAs(t, 'GET', `/data/suspend_note/${noteId}`); - expect(note.status).toBe(200); - const rec = ((await note.json()) as any).record ?? {}; - expect(rec.status).toBe('resolved'); - expect(rec.resolution).toBe('fixed upstream'); - } finally { - await cold.stop(); - } - }, 120_000); + it('the durable row is what a rehydration would read — the whole SuspendedRun round-trips', async () => { + // The cold-boot half of #4470's ask cannot be asserted from this harness + // yet: file-backed sqlite-wasm data does not survive an in-process kernel + // restart here, so a second `bootStack` over the same file reads created + // tables with no rows (filed as a blocker — see the note at the end of this + // file). Rather than assert nothing, this pins the thing that failure mode + // would actually destroy: that every field `AutomationEngine` needs to + // REBUILD the pause is present and correctly shaped in the persisted row. + // + // That is exactly what #4420 lacked. Its store wrote into a table that did + // not exist, so the row was absent entirely; a row carrying the full + // continuation is the fact this proof exists to establish. + const row = await hot!.apiAs(hotToken, 'GET', `/data/sys_automation_run/${runId}`); + expect(row.status).toBe(200); + const rec = ((await row.json()) as any).record ?? {}; + + // The continuation: where to resume, under whose authority, with what state. + expect(rec.flow_name).toBe('flow_durable_suspend'); + expect(rec.node_id).toBe('ask'); + expect(rec.node_type).toBe('screen'); + expect(rec.status).toBe('paused'); + expect(rec.started_at).toBeTruthy(); + + // The variable snapshot and the step log both have to be parseable JSON — + // a store that stringified them wrongly would round-trip into a run whose + // downstream nodes see no variables at all. + const vars = JSON.parse(String(rec.variables_json)); + expect(vars.noteId).toBe(noteId); + expect(Array.isArray(JSON.parse(String(rec.steps_json)))).toBe(true); + + // `screen_json` is what a rehydrated pause validates a resume against + // (#4477), so the declared field contract must survive persistence too. + const screen = JSON.parse(String(rec.screen_json)); + expect(screen.nodeId).toBe('ask'); + expect(screen.fields.map((f: any) => f.name)).toContain('resolution'); + expect(screen.fields.find((f: any) => f.name === 'resolution').required).toBe(true); + }); + + it('the suspension is CONSUMED from the durable store on resume — no zombie row is left behind', async () => { + // The other half of durability, and the one #4420 turned into zombies: a + // row that outlives its run is a request the approvals sweep would later + // have to reason about. Resuming must delete it, not merely stop reading it. + const resumed = await hot!.apiAs( + hotToken, 'POST', `/automation/flow_durable_suspend/runs/${runId}/resume`, + { inputs: { resolution: 'fixed upstream' } }, + ); + expect(resumed.status, await resumed.clone().text()).toBeLessThan(300); + + // The downstream node ran with the snapshotted variable. + const note = await hot!.apiAs(hotToken, 'GET', `/data/suspend_note/${noteId}`); + const rec = ((await note.json()) as any).record ?? {}; + expect(rec.status).toBe('resolved'); + expect(rec.resolution).toBe('fixed upstream'); + + // The `paused` row is gone… + const gone = await hot!.apiAs(hotToken, 'GET', `/data/sys_automation_run/${runId}`); + expect(gone.status).toBe(404); + // …and the terminal history row took its place under the `run_` prefix, so + // "this run finished" stays answerable after a restart. + const history = await hot!.apiAs(hotToken, 'GET', `/data/sys_automation_run/run_${runId}`); + expect(history.status).toBe(200); + expect((((await history.json()) as any).record ?? {}).status).toBe('completed'); + }); - it('the screen contract is enforced on a rehydrated pause too (#4477 over the durable path)', async () => { - // A fresh kernel and a fresh run: the field contract has to survive - // persistence as well, since `screen_json` is what a rehydrated pause - // validates against. Suspend in one process, decide in the next. - const first = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); - let secondRun: string; - try { - const t = await first.signIn(); - const created = await first.apiAs(t, 'POST', '/data/suspend_note', { name: 'n2', status: 'new' }); - const id = ((await created.json()) as any).id ?? ((await created.clone().json()) as any).record?.id; - - const triggered = await first.apiAs(t, 'POST', '/automation/flow_durable_suspend/trigger', { - params: { noteId: id }, - }); - const tj = (await triggered.json()) as any; - const result = tj.result ?? tj.data ?? tj; - expect(result.status).toBe('paused'); - secondRun = result.runId; - } finally { - await first.stop(); - } - - const cold = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); - try { - const t = await cold.signIn(); - const bad = await cold.apiAs( - t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun!}/resume`, { inputs: {} }, - ); - expect(bad.status).toBe(400); - expect(await bad.text()).toContain('resolution'); - - // Refused, not consumed: the legitimate submission still lands, in the - // same cold process. - const good = await cold.apiAs( - t, 'POST', `/automation/flow_durable_suspend/runs/${secondRun!}/resume`, - { inputs: { resolution: 'ok' } }, - ); - expect(good.status, await good.clone().text()).toBeLessThan(300); - } finally { - await cold.stop(); - } - }, 180_000); + it('enforces the screen contract on a run whose pause is durably stored (#4477 over the durable path)', async () => { + const created = await hot!.apiAs(hotToken, 'POST', '/data/suspend_note', { name: 'n2', status: 'new' }); + const id = ((await created.json()) as any).id; + const triggered = await hot!.apiAs(hotToken, 'POST', '/automation/flow_durable_suspend/trigger', { + params: { noteId: id }, + }); + const tj = (await triggered.json()) as any; + const second = (tj.result ?? tj.data ?? tj).runId; + + const bad = await hot!.apiAs( + hotToken, 'POST', `/automation/flow_durable_suspend/runs/${second}/resume`, { inputs: {} }, + ); + expect(bad.status).toBe(400); + expect(await bad.text()).toContain('resolution'); + + // Refused, not consumed — the durable row is still there and the + // legitimate submission still lands. + const still = await hot!.apiAs(hotToken, 'GET', `/data/sys_automation_run/${second}`); + expect(still.status).toBe(200); + const good = await hot!.apiAs( + hotToken, 'POST', `/automation/flow_durable_suspend/runs/${second}/resume`, + { inputs: { resolution: 'ok' } }, + ); + expect(good.status, await good.clone().text()).toBeLessThan(300); + }); }); diff --git a/packages/qa/dogfood/test/zz-probe2.dogfood.test.ts b/packages/qa/dogfood/test/zz-probe2.dogfood.test.ts deleted file mode 100644 index d4582487b9..0000000000 --- a/packages/qa/dogfood/test/zz-probe2.dogfood.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import { mkdtempSync, rmSync, statSync, appendFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { bootStack } from '@objectstack/verify'; -import { durableSuspendStack } from './fixtures/flow-durable-suspend-fixture.js'; - -const OUT = '/tmp/claude-0/-home-user/74049b9a-bd02-54d0-a5c5-c545cef3d7b6/scratchpad/probe2-out.txt'; -const LOG = (...a: any[]) => appendFileSync(OUT, a.map(String).join(' ') + '\n'); - -describe('probe2 — does ORDINARY data survive a harness restart?', () => { - it('probe2', async () => { - const dir = mkdtempSync(join(tmpdir(), 'os-probe2-')); - const dbFile = join(dir, 'v.sqlite'); - - const a = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); - const ta = await a.signIn(); - const c = await a.apiAs(ta, 'POST', '/data/suspend_note', { name: 'persisted-me', status: 'new' }); - LOG('probe2 create status', c.status); - LOG('probe2 size before stop', statSync(dbFile).size); - await a.stop(); - LOG('probe2 size after stop', statSync(dbFile).size); - - const b = await bootStack(durableSuspendStack, { automation: true, databaseFile: dbFile }); - const tb = await b.signIn(); - const list = await b.apiAs(tb, 'GET', '/data/suspend_note?limit=10'); - LOG('probe2 cold notes', (await list.text()).slice(0, 400)); - await b.stop(); - - rmSync(dir, { recursive: true, force: true }); - expect(true).toBe(true); - }, 180_000); -});