diff --git a/DECISION-LOGS.md b/DECISION-LOGS.md index c2df150c7..a756d75cf 100644 --- a/DECISION-LOGS.md +++ b/DECISION-LOGS.md @@ -6,14 +6,22 @@ - _08.04.2025_: [Lazy-loaded Icons](./apps/icons/lazy-loaded-icons-08-04-2025.decision-log.md) - _15.04.2025_: [Internationalization implementation with i18next](./packages/sdk/src/features/i18n/i18next.decision-log.md) - _26.05.2025_: [JSON Form Validation Strategy](./packages/sdk/src/features/json-form/form-validation.decision-log.md) +- _05.03.2026_: [Independent docs deployment strategy](./apps/docs/docs-deployment.decision-log.md) +- _13.03.2026_: [Remark plugin for automatic base path link rewriting](./apps/docs/remark-base-path-links.decision-log.md) - _16.04.2026_: [CSP-safe Ajv replacement with @cfworker/json-schema](./packages/sdk/src/utils/validation/workflow-builder-validator-16-04-2026.decision-log.md) - _22.04.2026_: [SDK restructuring — inversion, relocation, plugin API, config naming](./packages/sdk/sdk-restructuring.decision-log.md) - _27.04.2026_: [Default to 127.0.0.1 binding for the reference backend](./apps/backend/local-dev-binding.decision-log.md) - _27.04.2026_: [Workflow cancellation handling in Temporal engine](./packages/temporal/src/workflow/cancellation-handling.decision-log.md) - _28.04.2026_: [Topological scheduling for the graph runner](./packages/execution-core/topological-scheduling.decision-log.md) - _29.04.2026_: [Decision executor fails fast on no matching branch](./packages/execution-core/decision-no-match.decision-log.md) +- _30.04.2026 (revised 04.05.2026 after team review)_: [Audience-based docs IA + schema authoring reference](./apps/docs/docs-restructure.decision-log.md) +- _30.04.2026_: [TypeDoc-driven API Reference for `@workflowbuilder/sdk`](./apps/docs/typedoc-api-reference.decision-log.md) - _05.05.2026_: [Extract AI Studio from `apps/demo` into its own `apps/ai-studio` app](./apps/ai-studio/ai-studio-extraction.decision-log.md) - _05.05.2026_: [Workspace layout — relocate libraries to `packages/`](./packages/sdk/workspace-layout.decision-log.md) - _06.05.2026_: [Make execution-core generic over the consumer's node union](./packages/execution-core/generic-execution-core.decision-log.md) - _15.05.2026_: [AuthPort seam for backend authn/authz](./apps/backend/auth-port.decision-log.md) +- _21.05.2026 (proposed; landed 03.06.2026 in `fa5999dd`)_: [TenantContextPort — multi-tenant identity seam for the reference backend](./apps/backend/tenant-context-port.decision-log.md) +- _07.08.2026_: [Keep the postcss box-sizing plugin over lint-based or selector-based alternatives](./packages/ui/postcss-box-sizing.decision-log.md) - _24.08.2026_: [`incomplete` as a third terminal state, distinct from `failed` and from a stall](./packages/execution-core/terminal-states.decision-log.md) +- _07.09.2026 (shape), 08.09.2026 (names)_: [Decision request as versioned data on a node](./apps/backend/decision-request.decision-log.md) +- _08.09.2026_: [Durable pause, the Temporal side of the human-in-the-loop seam](./packages/temporal/src/workflow/durable-pause.decision-log.md) diff --git a/apps/backend/README.md b/apps/backend/README.md index fdd487c41..7e932a9d3 100644 --- a/apps/backend/README.md +++ b/apps/backend/README.md @@ -29,6 +29,18 @@ Frontend (React) - **Domain** (`packages/execution-core`) — pure graph runner + ports + node executors. No Temporal, no HTTP. See the [execution-core README](../../packages/execution-core/README.md). - **Frontend** (`apps/ai-studio`) — full AI workflow product. Composes `@workflowbuilder/sdk` directly via JSX, with a slim plugin only for per-node execution markers. Owns Play/Stop controls, log panel, node detail, and execution highlighting. +## Decision request on a node + +A node asks a human for a decision by carrying `data.properties.decisionRequest`: the actions offered, the JSON Schema of the form, the node whose output is judged, and an optional deadline. Any node type may carry one: the backend and the decision endpoint find the request by this field, never by `type`. The mapper lifts it to `BaseNode.decisionRequest`, out of `config`. + +The runner does not read the field, deliberately: it learns no product's vocabulary, so a run stops where a node's executor returns a waiting result. A request on a node that never parks therefore validates, reaches the worker and asks nobody anything. The node whose executor does nothing but park is its own task, listed under what this change leaves out. + +The request is validated on `POST /:id/publish` and `POST /:id/execute`, never on `PATCH /:id/draft`: a draft is legitimately mid-edit. A broken request answers with the existing `invalid_snapshot` 400, whose `details[].path` points at the node index and field, for example `nodes.1.data.properties.decisionRequest.actions.1.effect`. Structural issues come first; the graph rules (proposal source, predecessors) run once the structure parses, so a second round of issues can follow a fix. Every domain message the validation can produce is listed in `src/domain/decision/decision-issues.ts`. Each such detail also carries `domainCode`, its key in that dictionary, and `params` with the value the message interpolates, so a client branches and translates on the identifier and never on the wording; `code` stays zod's own. + +One key is refused outright, wherever it sits. An own `__proto__` anywhere in the snapshot answers `invalid_snapshot` 400 naming its path: `JSON.parse` turns it into an ordinary key, and a loose object copies unknown keys by assignment, which for that one swaps the parsed output's prototype and hands the engine a request no schema ever saw. The check does not weigh position, so it also refuses a `__proto__` buried inside an opaque node property, where zod never copies keys one by one and the key is inert. A node type that keeps a raw JSON document in `data.properties` therefore cannot carry one. + +A submitted decision is checked against the request by `validateSubmittedDecision` in `src/domain/decision/`; the decision endpoint that calls it is a separate change. Shape, rules and the reasoning are in [`decision-request.decision-log.md`](./decision-request.decision-log.md). + ## Running individual processes For debugging, the parts that `pnpm dev:ai-studio` orchestrates can also be run separately: diff --git a/apps/backend/decision-request.decision-log.md b/apps/backend/decision-request.decision-log.md new file mode 100644 index 000000000..63c1b6c80 --- /dev/null +++ b/apps/backend/decision-request.decision-log.md @@ -0,0 +1,60 @@ +### Title: Decision request as versioned data on a node + +### Proposed by: Piotr Błaszczyk + +### Date: 07.09.2026 (shape), 08.09.2026 (names) + +## Context + +A run can park at a node until a person decides. The backend needs to know what a decision at that node looks like: the actions offered, the fields the decider sees and may correct, whose output is judged, how long to wait. Products bring their own vocabulary; the engine has a closed set of things it can do; the backend knows no product's node types. + +The shape itself is documented on the type (`packages/types/src/workflow-execution/decision-request.ts`) and enforced by `decisionRequestSchema` in `apps/backend/src/domain/decision/`. This log keeps only what the code cannot say. A complete example, the refund story from the design workshop, is the `workedExample()` fixture in `decision-request-schema.test.ts`. + +## Decision + +1. **Data on the node, not a node kind.** The request lives under the reserved key `data.properties.decisionRequest` and is lifted to `BaseNode.decisionRequest`, as `errorPolicy` is. Its presence is the only marker; nothing detects such a node by `type`. Present or absent, never `null`: a `null` value is refused, so an editor that clears the request must remove the key. A client adding its own node type never has to teach the backend about it. +2. **Name and effect are split.** `name` and `label` are the client's words ("Escalate to finance"); `effect` is the engine's closed set. A new business vocabulary is data, not a code change. +3. **Edit is not an action.** The decider corrects fields and approves. Whether a field may be edited is already said by `readOnly` in the schema; a second switch would be a second source of truth. `resume-with-edits` is therefore derived, never declared. +4. **JSON Schema for the form**, validated for shape only. The SDK already renders and validates JSON Schema, so the decision form comes for free. A real validator arrives with the first consumer that checks edited values `(follow-up: decision-edit-value-validation)`. Shape only means: an object with a `properties` map, `required` naming declared fields, and `readOnly`, `x-pii` and `type` well-typed where present. `type` is optional, as JSON Schema makes it and as JsonForms renders without it. Every other keyword passes through unread. +5. **Validated on publish and execute, never on draft save.** A draft is legitimately mid-edit; validating it would lose the author's work on every autosave. Both routes go through one `parseSnapshot` helper and the existing `invalid_snapshot` 400, so they cannot drift. Each domain issue in `details` carries `domainCode` and `params` beside zod's `code` and the English `message`, so a client keys on the identifier and the wording stays free to change. +6. **One definition of the proposal source.** `resolveProposalSource` is the only place that says which node's output a decision judges. The pending-decision resource and the rerun loop must call it rather than re-derive the rule. Publishing refuses a request whose source does not resolve: a published decision with nothing to judge is not what the author meant, and with no predecessor at all the node is an orphan the runner fails anyway. Only the rule that the source must not carry its own request stays tied to `rerun-source`, the one effect that re-runs it. A resolved source can still yield no proposal at decision time, when that branch was skipped, so the pending-decision resource keeps its no-proposal path. +7. **Read requests through the parser, never from raw JSON.** Only the parsed form carries the defaults (`port`, `reasonRequired`, `maxIterations`). The stored snapshot stays raw. +8. **Three names for the lifecycle.** `DecisionRequest` is what the node asks. `SubmittedDecision` is what the decider sends, still unchecked. `Decision` is what validation accepts and what is recorded on the node's completion and audited; it names the chosen action and carries the effect, edits, reason and comment. The matched `DecisionAction` is returned beside it for routing, never inside it, so the port and label live once, on the request. The shape of a submission is parsed with `submittedDecisionSchema` at the endpoint; `validateSubmittedDecision` assumes it and checks only the rules. The field is not called `decision` because it holds the question, not the answer, and not `decisionContract` because that reads as configuration rather than as a request to a person. +9. **Results carry `error?: undefined`, not an `ok` flag.** `{ value; error?: undefined } | { value?: undefined; error }` reads as plain error handling and the compiler still forbids both-set and neither-set. The flag only repeated what the presence of `error` says. +10. **Vocabulary.** A node carrying a request is a node; no separate noun names it. The rerun effect is named for what it does, `rerun-source`, never for what the source is. Action names in examples (`approve`, `reject`, `ask-again`) are the client's and await a sync with design. + +11. **An own `__proto__` key anywhere in a snapshot is refused before parsing.** `JSON.parse` makes it an ordinary key, and zod's loose objects copy unknown keys with a plain assignment, which for that key swaps the output's prototype: everything under it then reads back as validated, and the mapper would copy an inherited request into a real field on the way to the engine. Both parsers that preserve unknown keys are wrapped in a preprocess that rejects the key at its path: `workflowSnapshotSchema`, which answers the usual `invalid_snapshot` 400, and `decisionRequestSchema`, which guards itself so a caller parsing raw JSON with it cannot inherit a request no schema checked. `z.record` is immune by construction but cannot type known keys beside unknown ones, and it protects only its own level, so it is no substitute here. + +## Rejected + +- Detecting the node by its type string: the backend would have to learn every product's vocabulary. +- A home-grown field list instead of JSON Schema: a second standard to render and validate. +- An `ignore` verb: a disguised "abandon the run". +- Defaulting `deadline.policy` to `reject`: a timer that rejects is audit-relevant and must be written down, not implied. +- Recording the whole action object on the decision: the port and label would then live twice, on the request and in every completion, with two sources of truth about where a verdict routes. +- Exporting the duration pattern from the Temporal plugin: a published API widened for one regex; duplicated with a pointer instead `(follow-up: shared-duration-format)`. + +## Known gaps + +- A workflow with a `null` draft still publishes `null`, unvalidated, as it did before. Changing that is its own decision. +- A draft may store an own `__proto__` key; it goes nowhere but the database, and publish and execute refuse it. Rejecting it at save time was judged not worth touching the draft route. +- The submission validator returns the first refusal, not a list. +- It checks editability and presence at every level the form describes inline, following `properties` and `items`. A level reached only through `$ref` or a composition keyword describes nothing there, so an edit into it is refused as an unknown field rather than checked `(follow-up: decision-edit-schema-composition)`. +- The snapshot schema does not check that edge endpoints exist, so an explicit source with a dangling edge passes. This predates the change. +- Node ids are not checked for uniqueness either; with a duplicate, the graph rules see the first node of that id. Also pre-existing `(follow-up: snapshot-node-id-uniqueness)`. + +## Open points + +Taken conservatively; confirm or change when the decision endpoint lands. + +- A whitespace-only `reason` counts as missing when `reasonRequired` is set, like a blank comment. +- "Emptied" for a required field means `undefined`, `null` or a whitespace-only string; empty arrays and objects are value validation. +- Edits on a non-`resume` submission are checked but do not change the effect; refusing them with a dedicated code is the recommended alternative. + +## Not in this change + +Further request fields (condition, four-eyes, several decisions), identity and `x-pii` masking, the decision endpoint, the pending-decision resource, the rerun loop, the deadline timer, authoring the request in the editor `(follow-up: decision-request-properties-ui)`, and the node that actually parks. The runner learns no product's vocabulary by design, so a run stops where a node's executor returns a waiting result, never because a field is present. The node type whose executor does only that, and therefore waits without side effects of its own, is its own task `(follow-up: human-decision-node)`. + +## Status + +Accepted diff --git a/apps/backend/src/domain/decision/decision-issues.test.ts b/apps/backend/src/domain/decision/decision-issues.test.ts new file mode 100644 index 000000000..7c421b0db --- /dev/null +++ b/apps/backend/src/domain/decision/decision-issues.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from 'vitest'; + +import { + DECISION_ISSUE_MESSAGES, + decisionIssue, + decisionIssueMessage, + decisionIssueOf, + decisionRefinement, +} from './decision-issues'; + +describe('decisionIssueMessage', () => { + it('fills the placeholder', () => { + expect(decisionIssueMessage('duplicate_action_name', 'approve')).toBe( + "action name 'approve' is used more than once", + ); + }); + + it('keeps replacement patterns in the value verbatim', () => { + expect(decisionIssueMessage('source_has_decision_request', '$&-$1')).toBe( + "proposal source '$&-$1' carries its own decision request and cannot be re-run", + ); + }); + + it('ignores a value for a message without a placeholder', () => { + expect(decisionIssueMessage('resume_required', 'ignored')).toBe(DECISION_ISSUE_MESSAGES.resume_required); + }); + + it('builds the issue shape a superRefine adds, carrying its identifier', () => { + expect(decisionIssue('duplicate_action_name', ['actions', 1, 'name'], 'approve')).toEqual({ + code: 'custom', + message: "action name 'approve' is used more than once", + path: ['actions', 1, 'name'], + params: { issue: 'duplicate_action_name', value: 'approve' }, + }); + }); + + it('leaves `value` out of the identifier when the message has none', () => { + expect(decisionIssue('port_empty', ['actions', 0, 'port']).params).toEqual({ issue: 'port_empty' }); + expect(decisionRefinement('port_empty')).toEqual({ + error: 'port must not be blank', + params: { issue: 'port_empty' }, + }); + }); +}); + +describe('decisionIssueOf', () => { + it('reads the identifier back off an issue, with and without a value', () => { + expect(decisionIssueOf(decisionIssue('duplicate_effect', ['actions'], 'resume'))).toEqual({ + issue: 'duplicate_effect', + value: 'resume', + }); + expect(decisionIssueOf(decisionIssue('resume_required', ['actions']))).toEqual({ issue: 'resume_required' }); + }); + + it.each([ + { name: "zod's own structural issue", issue: { code: 'invalid_type', path: ['nodes'], message: 'x' } }, + { name: 'params that are not ours', issue: { code: 'custom', params: { minimum: 1 } } }, + { name: 'an identifier not in the dictionary', issue: { code: 'custom', params: { issue: 'made_up' } } }, + { + name: 'an identifier that exists only on Object.prototype', + issue: { code: 'custom', params: { issue: 'constructor' } }, + }, + { name: 'no object at all', issue: null }, + ])('answers undefined for $name', ({ issue }) => { + expect(decisionIssueOf(issue)).toBeUndefined(); + }); +}); diff --git a/apps/backend/src/domain/decision/decision-issues.ts b/apps/backend/src/domain/decision/decision-issues.ts new file mode 100644 index 000000000..78412216b --- /dev/null +++ b/apps/backend/src/domain/decision/decision-issues.ts @@ -0,0 +1,80 @@ +// Every message the decision-request validation can produce. `{value}` is the one +// interpolation slot. Structural failures (wrong type, missing key) keep zod's wording. +export const DECISION_ISSUE_MESSAGES = { + actions_empty: 'at least one action is required', + name_empty: 'name must not be blank', + label_empty: 'label must not be blank', + unknown_effect: 'effect must be one of {value}', + duplicate_action_name: "action name '{value}' is used more than once", + duplicate_effect: "only one action may have effect '{value}'", + resume_required: "an action with effect 'resume' is required", + port_empty: 'port must not be blank', + port_reserved: "port must not be the reserved 'errorRoute'", + reject_port_equals_resume_port: "reject port '{value}' must differ from the resume port", + required_field_undeclared: "required field '{value}' is not declared in properties", + deadline_format: + "must be a duration such as '30s' or '3d' (number plus ms, s, m, h or d), above zero and at most '3652500d'", + deadline_policy: "policy must be 'reject'", + source_node_without_decision_request: 'this node carries no decision request', + source_not_a_predecessor: "proposalSourceNodeId '{value}' is not a direct predecessor of this node", + source_missing: 'a rerun-source action needs a proposal source, but this node has no predecessor', + source_ambiguous: 'several predecessors; set proposalSourceNodeId to say which one rerun-source re-runs', + source_has_decision_request: "proposal source '{value}' carries its own decision request and cannot be re-run", +} as const; + +export type DecisionIssueCode = keyof typeof DECISION_ISSUE_MESSAGES; + +// Every way a submitted decision can be refused against the node's decision request. +export const SUBMITTED_DECISION_ERRORS = { + unknown_action: "the decision request offers no action named '{value}'", + reason_required: "action '{value}' requires a reason", + comment_required: "action '{value}' requires a comment", + unknown_field: "field '{value}' is not in the decision schema", + field_not_editable: "field '{value}' is read-only", + required_field_missing: "required field '{value}' must not be emptied", +} as const; + +export type SubmittedDecisionErrorCode = keyof typeof SUBMITTED_DECISION_ERRORS; + +function fill(template: string, value: string | undefined): string { + // A function replacer, so a value containing `$&` or `$1` lands verbatim. + return template.replace('{value}', () => value ?? ''); +} + +export function decisionIssueMessage(code: DecisionIssueCode, value?: string): string { + return fill(DECISION_ISSUE_MESSAGES[code], value); +} + +export function submittedDecisionErrorMessage(code: SubmittedDecisionErrorCode, value?: string): string { + return fill(SUBMITTED_DECISION_ERRORS[code], value); +} + +// Rides on the zod issue as `params` and is read back by the HTTP serializer, so a client +// branches and translates on the identifier and never on the wording of `message`. +export type DecisionIssueParams = { issue: DecisionIssueCode; value?: string }; + +function decisionIssueParams(code: DecisionIssueCode, value: string | undefined): DecisionIssueParams { + return value === undefined ? { issue: code } : { issue: code, value }; +} + +export function decisionIssue(code: DecisionIssueCode, path: PropertyKey[], value?: string) { + return { + code: 'custom' as const, + message: decisionIssueMessage(code, value), + path, + params: decisionIssueParams(code, value), + }; +} + +// The same issue, shaped as the options a `.refine` takes. +export function decisionRefinement(code: DecisionIssueCode, value?: string) { + return { error: decisionIssueMessage(code, value), params: decisionIssueParams(code, value) }; +} + +export function decisionIssueOf(issue: unknown): DecisionIssueParams | undefined { + const params = typeof issue === 'object' && issue !== null ? (issue as { params?: unknown }).params : undefined; + if (typeof params !== 'object' || params === null) return undefined; + const { issue: code, value } = params as { issue?: unknown; value?: unknown }; + if (typeof code !== 'string' || !Object.hasOwn(DECISION_ISSUE_MESSAGES, code)) return undefined; + return decisionIssueParams(code as DecisionIssueCode, typeof value === 'string' ? value : undefined); +} diff --git a/apps/backend/src/domain/decision/decision-request-schema.test.ts b/apps/backend/src/domain/decision/decision-request-schema.test.ts new file mode 100644 index 000000000..8686dc963 --- /dev/null +++ b/apps/backend/src/domain/decision/decision-request-schema.test.ts @@ -0,0 +1,423 @@ +import { describe, expect, expectTypeOf, it } from 'vitest'; +import type { z } from 'zod'; + +import { + DECLARABLE_DECISION_EFFECTS, + type DecisionRequest, +} from '@workflow-builder/types/workflow-execution/decision-request'; + +import { type DecisionIssueCode, decisionIssueMessage, decisionIssueOf } from './decision-issues'; +import { decisionRequestSchema } from './decision-request-schema'; + +const approve = { name: 'approve', label: 'Approve', effect: 'resume', port: 'approved' }; +const reject = { name: 'reject', label: 'Reject', effect: 'reject', port: 'rejected', reasonRequired: false }; +const askAgain = { name: 'ask-again', label: 'Ask again', effect: 'rerun-source', maxIterations: 3 }; + +const refundForm = { + type: 'object', + properties: { + orderDate: { type: 'string', title: 'Order date', readOnly: true }, + customerEmail: { type: 'string', title: 'Customer e-mail', readOnly: true, 'x-pii': true }, + refundAmount: { type: 'number', title: 'Refund amount' }, + emailDraft: { type: 'string', title: 'E-mail draft' }, + }, + required: ['refundAmount'], +}; + +// The refund story from the design workshop. +function workedExample() { + return { + version: 1, + actions: [approve, reject, askAgain], + schema: refundForm, + uiSchema: { type: 'VerticalLayout', elements: [] }, + proposalSourceNodeId: 'source-1', + deadline: { after: '3d', policy: 'reject' }, + }; +} + +function request(overrides: Record = {}): unknown { + return { ...workedExample(), ...overrides }; +} + +function issuesOf(input: unknown): { path: string; message: string; domain?: { issue: string; value?: string } }[] { + const result = decisionRequestSchema.safeParse(input); + return result.success + ? [] + : result.error.issues.map((issue) => ({ + path: issue.path.join('.'), + message: issue.message, + domain: decisionIssueOf(issue), + })); +} + +const declarableEffects = DECLARABLE_DECISION_EFFECTS.join(', '); + +describe('decisionRequestSchema', () => { + it('accepts the refund worked example', () => { + expect(decisionRequestSchema.safeParse(workedExample()).success).toBe(true); + }); + + it('accepts a minimal request: one resume action and an empty form', () => { + const minimal = { + version: 1, + actions: [{ name: 'ok', label: 'OK', effect: 'resume' }], + schema: { type: 'object', properties: {} }, + }; + + expect(decisionRequestSchema.safeParse(minimal).success).toBe(true); + }); + + // Shapes JsonForms 3.5.1 generates a control for. The parser reads none of their + // keywords, so each has to reach the renderer exactly as authored. + it.each([ + { shape: 'a list of type names', property: { type: ['string', 'null'] } }, + { shape: 'enum alone', property: { enum: ['open', 'closed'] } }, + { shape: 'a local $ref', property: { $ref: '#/$defs/money' } }, + { shape: 'anyOf alone', property: { anyOf: [{ type: 'string' }, { type: 'number' }] } }, + { shape: 'readOnly beside no type', property: { enum: ['open'], readOnly: true, 'x-pii': true } }, + ])('accepts a form property declared with $shape, untouched', ({ property }) => { + const parsed = decisionRequestSchema.safeParse( + request({ schema: { type: 'object', properties: { field: property } } }), + ); + + expect(parsed.success).toBe(true); + expect(parsed.success ? parsed.data.schema['properties'] : undefined).toEqual({ field: property }); + }); + + it.each([...DECLARABLE_DECISION_EFFECTS])('accepts a declared %s action', (effect) => { + const resume = { name: 'ok', label: 'OK', effect: 'resume' }; + const actions = effect === 'resume' ? [resume] : [resume, { name: 'other', label: 'Other', effect }]; + + expect(decisionRequestSchema.safeParse(request({ actions })).success).toBe(true); + }); + + it('materialises the defaults for port, reasonRequired and maxIterations', () => { + const parsed = decisionRequestSchema.parse( + request({ + actions: [ + { name: 'approve', label: 'Approve', effect: 'resume' }, + { name: 'reject', label: 'Reject', effect: 'reject' }, + { name: 'ask-again', label: 'Ask again', effect: 'rerun-source' }, + ], + }), + ); + + expect(parsed.actions).toEqual([ + { name: 'approve', label: 'Approve', effect: 'resume', port: 'approved' }, + { name: 'reject', label: 'Reject', effect: 'reject', port: 'rejected', reasonRequired: false }, + { name: 'ask-again', label: 'Ask again', effect: 'rerun-source', maxIterations: 3 }, + ]); + }); + + it('keeps unknown keys at every level', () => { + const input = { + ...workedExample(), + audience: 'finance', + actions: [{ ...approve, icon: 'check' }], + schema: { + ...refundForm, + $schema: 'https://json-schema.org/draft/2020-12/schema', + properties: { + ...refundForm.properties, + customerEmail: { ...refundForm.properties.customerEmail, 'x-mask': 'email' }, + }, + }, + uiSchema: { type: 'VerticalLayout', elements: [{ type: 'Control', scope: '#/properties/refundAmount' }] }, + deadline: { after: '3d', policy: 'reject', warnAfter: '2d' }, + }; + + const parsed = decisionRequestSchema.parse(input); + + expect(parsed).toMatchObject({ + audience: 'finance', + actions: [{ icon: 'check' }], + schema: { + $schema: 'https://json-schema.org/draft/2020-12/schema', + properties: { customerEmail: { title: 'Customer e-mail', 'x-mask': 'email' } }, + }, + uiSchema: input.uiSchema, + deadline: { warnAfter: '2d' }, + }); + }); + + it('accepts an explicit readOnly: false', () => { + const schema = { type: 'object', properties: { amount: { type: 'number', readOnly: false } } }; + + const parsed = decisionRequestSchema.parse(request({ schema })); + + expect(parsed.schema.properties['amount']).toEqual({ type: 'number', readOnly: false }); + }); + + it.each(['100ms', '30s', '10m', '1.5h', '24h', '3d', '7d', '3652500d'])('accepts a deadline of %s', (after) => { + expect(decisionRequestSchema.safeParse(request({ deadline: { after, policy: 'reject' } })).success).toBe(true); + }); + + it('accepts a request without deadline, uiSchema or proposalSourceNodeId', () => { + const { version, actions, schema } = workedExample(); + + expect(decisionRequestSchema.safeParse({ version, actions, schema }).success).toBe(true); + }); + + // `issue` names the dictionary entry expected at `path`; rows without one fail on zod's + // own structural check. + it.each<{ name: string; input: unknown; path: string; issue?: { code: DecisionIssueCode; value?: string } }>([ + { name: 'a version other than 1', input: request({ version: 2 }), path: 'version' }, + { + name: 'an empty action list', + input: request({ actions: [] }), + path: 'actions', + issue: { code: 'actions_empty' }, + }, + { + name: 'a duplicate action name', + input: request({ actions: [approve, { ...reject, name: 'approve' }] }), + path: 'actions.1.name', + issue: { code: 'duplicate_action_name', value: 'approve' }, + }, + { + name: 'an effect outside the declarable set', + input: request({ actions: [{ ...approve, effect: 'escalate' }] }), + path: 'actions.0.effect', + issue: { code: 'unknown_effect', value: declarableEffects }, + }, + { + name: "a declared 'resume-with-edits'", + input: request({ actions: [approve, { ...reject, effect: 'resume-with-edits' }] }), + path: 'actions.1.effect', + issue: { code: 'unknown_effect', value: declarableEffects }, + }, + { + name: 'no resume action', + input: request({ actions: [reject] }), + path: 'actions', + issue: { code: 'resume_required' }, + }, + { + name: 'two resume actions', + input: request({ actions: [approve, { ...approve, name: 'approve-2' }] }), + path: 'actions.1.effect', + issue: { code: 'duplicate_effect', value: 'resume' }, + }, + { + name: 'two reject actions', + input: request({ actions: [approve, reject, { ...reject, name: 'decline' }] }), + path: 'actions.2.effect', + issue: { code: 'duplicate_effect', value: 'reject' }, + }, + { + name: 'two rerun-source actions', + input: request({ actions: [approve, askAgain, { ...askAgain, name: 'retry' }] }), + path: 'actions.2.effect', + issue: { code: 'duplicate_effect', value: 'rerun-source' }, + }, + { + name: 'a whitespace-only action name', + input: request({ actions: [{ ...approve, name: ' ' }] }), + path: 'actions.0.name', + issue: { code: 'name_empty' }, + }, + { + name: 'a whitespace-only action label', + input: request({ actions: [{ ...approve, label: ' ' }] }), + path: 'actions.0.label', + issue: { code: 'label_empty' }, + }, + { + name: 'a whitespace-only resume port', + input: request({ actions: [{ ...approve, port: '\t' }] }), + path: 'actions.0.port', + issue: { code: 'port_empty' }, + }, + { + name: 'an empty action name', + input: request({ actions: [{ ...approve, name: '' }] }), + path: 'actions.0.name', + issue: { code: 'name_empty' }, + }, + { + name: 'an empty action label', + input: request({ actions: [{ ...approve, label: '' }] }), + path: 'actions.0.label', + issue: { code: 'label_empty' }, + }, + { + name: 'an empty resume port', + input: request({ actions: [{ ...approve, port: '' }] }), + path: 'actions.0.port', + issue: { code: 'port_empty' }, + }, + { + name: "a resume port of 'errorRoute'", + input: request({ actions: [{ ...approve, port: 'errorRoute' }] }), + path: 'actions.0.port', + issue: { code: 'port_reserved' }, + }, + { + name: "a reject port of 'errorRoute'", + input: request({ actions: [approve, { ...reject, port: 'errorRoute' }] }), + path: 'actions.1.port', + issue: { code: 'port_reserved' }, + }, + { + name: 'a reject port equal to the resume port', + input: request({ actions: [approve, { ...reject, port: 'approved' }] }), + path: 'actions.1.port', + issue: { code: 'reject_port_equals_resume_port', value: 'approved' }, + }, + { + name: 'a non-boolean reasonRequired', + input: request({ actions: [approve, { ...reject, reasonRequired: 'yes' }] }), + path: 'actions.1.reasonRequired', + }, + { + name: 'maxIterations below 1', + input: request({ actions: [approve, { ...askAgain, maxIterations: 0 }] }), + path: 'actions.1.maxIterations', + }, + { + name: 'a fractional maxIterations', + input: request({ actions: [approve, { ...askAgain, maxIterations: 1.5 }] }), + path: 'actions.1.maxIterations', + }, + { + name: "a form schema whose type is not 'object'", + input: request({ schema: { ...refundForm, type: 'array' } }), + path: 'schema.type', + }, + { name: 'no form schema at all', input: request({ schema: undefined }), path: 'schema' }, + { + name: 'a form schema without properties', + input: request({ schema: { type: 'object' } }), + path: 'schema.properties', + }, + { + name: 'a form property whose type is neither a name nor a list of names', + input: request({ schema: { type: 'object', properties: { refundAmount: { type: 7 } } } }), + path: 'schema.properties.refundAmount.type', + }, + { + name: 'a form property whose type list holds something other than a name', + input: request({ schema: { type: 'object', properties: { refundAmount: { type: ['string', 7] } } } }), + path: 'schema.properties.refundAmount.type', + }, + { + name: 'a non-boolean readOnly', + input: request({ schema: { type: 'object', properties: { orderDate: { type: 'string', readOnly: 'true' } } } }), + path: 'schema.properties.orderDate.readOnly', + }, + { + name: 'a non-boolean x-pii', + input: request({ schema: { type: 'object', properties: { email: { type: 'string', 'x-pii': 'yes' } } } }), + path: 'schema.properties.email.x-pii', + }, + { + name: 'a required field that is not declared', + input: request({ schema: { ...refundForm, required: ['discount'] } }), + path: 'schema.required.0', + issue: { code: 'required_field_undeclared', value: 'discount' }, + }, + { + name: 'a required field that exists only on Object.prototype', + input: request({ schema: { ...refundForm, required: ['constructor'] } }), + path: 'schema.required.0', + issue: { code: 'required_field_undeclared', value: 'constructor' }, + }, + { + name: 'a deadline without a unit', + input: request({ deadline: { after: '3', policy: 'reject' } }), + path: 'deadline.after', + issue: { code: 'deadline_format' }, + }, + { + name: 'a deadline of zero', + input: request({ deadline: { after: '0s', policy: 'reject' } }), + path: 'deadline.after', + issue: { code: 'deadline_format' }, + }, + { + name: 'a negative deadline', + input: request({ deadline: { after: '-5m', policy: 'reject' } }), + path: 'deadline.after', + issue: { code: 'deadline_format' }, + }, + { + name: 'a deadline beyond the protobuf Duration range', + input: request({ deadline: { after: '3652501d', policy: 'reject' } }), + path: 'deadline.after', + issue: { code: 'deadline_format' }, + }, + { name: 'a deadline without a policy', input: request({ deadline: { after: '3d' } }), path: 'deadline.policy' }, + { + name: "a deadline policy other than 'reject'", + input: request({ deadline: { after: '3d', policy: 'escalate' } }), + path: 'deadline.policy', + issue: { code: 'deadline_policy' }, + }, + { name: 'a uiSchema that is not an object', input: request({ uiSchema: 'vertical' }), path: 'uiSchema' }, + { + name: 'a non-string proposalSourceNodeId', + input: request({ proposalSourceNodeId: 42 }), + path: 'proposalSourceNodeId', + }, + ])('rejects $name', ({ input, path, issue }) => { + const issues = issuesOf(input); + const atPath = issues.filter((candidate) => candidate.path === path); + + expect(decisionRequestSchema.safeParse(input).success).toBe(false); + expect(atPath.length).toBeGreaterThan(0); + if (issue !== undefined) { + expect(atPath.map((candidate) => candidate.message)).toContain(decisionIssueMessage(issue.code, issue.value)); + // The identifier, not the wording, is what a client keys on. + expect(atPath.map((candidate) => candidate.domain)).toContainEqual( + issue.value === undefined ? { issue: issue.code } : { issue: issue.code, value: issue.value }, + ); + } + }); + + // The effect check aborts the action the way the union's own failure did; a second issue + // about a missing resume action for an action that never parsed would only mislead. + it('reports an unknown effect once, without a missing-resume issue riding along', () => { + const issues = issuesOf(request({ actions: [{ name: 'a', label: 'A', effect: 'zzz' }] })); + + expect(issues.map((issue) => issue.path)).toEqual(['actions.0.effect']); + expect(issues[0]?.domain).toEqual({ issue: 'unknown_effect', value: declarableEffects }); + }); + + it('parses into a value assignable to DecisionRequest', () => { + expectTypeOf>().toMatchTypeOf(); + }); +}); + +// Every level of a request is a loose object, and a loose object copies an unknown key by +// assignment, which for this one swaps the parsed output's prototype. The schema guards +// itself rather than relying on the snapshot it is usually nested in. +describe('decisionRequestSchema: own __proto__ keys, parsed on its own', () => { + const action = '{"name":"approve","label":"Approve","effect":"resume"}'; + const form = '{"type":"object","properties":{}}'; + + it.each([ + { + where: 'at the request level', + json: `{"version":1,"actions":[${action}],"schema":${form},"__proto__":{"deadline":{"after":"nonsense"}}}`, + path: '__proto__', + }, + { + where: 'inside an action', + json: `{"version":1,"actions":[{"name":"a","label":"A","effect":"resume","__proto__":{"port":"stolen"}}],"schema":${form}}`, + path: 'actions.0.__proto__', + }, + { + where: 'inside the form schema', + json: `{"version":1,"actions":[${action}],"schema":{"type":"object","properties":{},"__proto__":{"required":["x"]}}}`, + path: 'schema.__proto__', + }, + { + where: 'inside a form property', + json: `{"version":1,"actions":[${action}],"schema":{"type":"object","properties":{"amount":{"type":"number","__proto__":{"readOnly":true}}}}}`, + path: 'schema.properties.amount.__proto__', + }, + ])('refuses one $where', ({ json, path }) => { + expect(issuesOf(JSON.parse(json))).toEqual([{ path, message: "the key '__proto__' is not allowed" }]); + }); +}); diff --git a/apps/backend/src/domain/decision/decision-request-schema.ts b/apps/backend/src/domain/decision/decision-request-schema.ts new file mode 100644 index 000000000..46310eda4 --- /dev/null +++ b/apps/backend/src/domain/decision/decision-request-schema.ts @@ -0,0 +1,150 @@ +import { z } from 'zod'; + +import { DECLARABLE_DECISION_EFFECTS } from '@workflow-builder/types/workflow-execution/decision-request'; + +import { rejectingOwnProtoKey } from '../schema/own-proto-key'; +import { decisionIssue, decisionRefinement } from './decision-issues'; + +// Mirrors DURATION_PATTERN and the protobuf Duration range in +// packages/temporal/src/workflow/profile-validation.ts (follow-up: shared-duration-format) +const DURATION_PATTERN = /^(\d+(?:\.\d+)?)(ms|s|m|h|d)$/; +const UNIT_MS = { ms: 1, s: 1000, m: 60_000, h: 3_600_000, d: 86_400_000 } as const; +const MIN_DURATION_MS = 0.000_001; +const MAX_DURATION_MS = 315_576_000_000 * UNIT_MS.s; + +function isDurationString(value: string): boolean { + const match = DURATION_PATTERN.exec(value); + if (match === null) return false; + const milliseconds = Number.parseFloat(match[1]) * UNIT_MS[match[2] as keyof typeof UNIT_MS]; + return milliseconds >= MIN_DURATION_MS && milliseconds <= MAX_DURATION_MS; +} + +const durationSchema = z.string().refine(isDurationString, decisionRefinement('deadline_format')); + +function isNotBlank(text: string): boolean { + return text.trim().length > 0; +} + +// 'errorRoute' is the handle the runner reserves for the error policy. +const portSchema = z + .string() + .refine(isNotBlank, decisionRefinement('port_empty')) + .refine((port) => port !== 'errorRoute', decisionRefinement('port_reserved')); + +const actionBase = { + name: z.string().refine(isNotBlank, decisionRefinement('name_empty')), + label: z.string().refine(isNotBlank, decisionRefinement('label_empty')), +}; + +const resumeActionSchema = z.looseObject({ + ...actionBase, + effect: z.literal('resume'), + port: portSchema.default('approved'), +}); + +const rejectActionSchema = z.looseObject({ + ...actionBase, + effect: z.literal('reject'), + port: portSchema.default('rejected'), + reasonRequired: z.boolean().default(false), +}); + +const rerunSourceActionSchema = z.looseObject({ + ...actionBase, + effect: z.literal('rerun-source'), + maxIterations: z.int().min(1).default(3), +}); + +// The effect picks the member that parses the rest, so it is checked on its own first: a +// union that finds no member cannot name what was wrong, and a client needs the name. +const declaredEffect = z.unknown().superRefine((action, context) => { + if (typeof action !== 'object' || action === null || Array.isArray(action)) return; + const effect = (action as { effect?: unknown }).effect; + if (typeof effect === 'string' && (DECLARABLE_DECISION_EFFECTS as readonly string[]).includes(effect)) return; + // Aborting, as the union's own failure was: the request-level rules must not go on to + // report a missing resume action for an action that never parsed. + context.addIssue({ + ...decisionIssue('unknown_effect', ['effect'], DECLARABLE_DECISION_EFFECTS.join(', ')), + continue: false, + }); +}); + +const decisionActionSchema = declaredEffect.pipe( + z.discriminatedUnion('effect', [resumeActionSchema, rejectActionSchema, rerunSourceActionSchema]), +); + +const formPropertySchema = z.looseObject({ + type: z.union([z.string(), z.array(z.string())]).optional(), + readOnly: z.boolean().optional(), + 'x-pii': z.boolean().optional(), +}); + +// Shape only. +const formSchema = z + .looseObject({ + type: z.literal('object'), + properties: z.record(z.string(), formPropertySchema), + required: z.array(z.string()).optional(), + }) + .superRefine((form, context) => { + for (const [index, name] of (form.required ?? []).entries()) { + if (!Object.hasOwn(form.properties, name)) { + context.addIssue(decisionIssue('required_field_undeclared', ['required', index], name)); + } + } + }); + +const deadlineSchema = z.looseObject({ + after: durationSchema, + policy: z.string().refine((policy) => policy === 'reject', decisionRefinement('deadline_policy')), +}); + +// Guarded on its own, not only through `workflowSnapshotSchema`: every level below is a +// loose object, so a caller parsing raw JSON with this export would inherit a request no +// schema checked. `../schema/own-proto-key.ts` says why a loose object needs that. +export const decisionRequestSchema = rejectingOwnProtoKey( + z + .looseObject({ + version: z.literal(1), + actions: z + .array(decisionActionSchema) + .refine((actions) => actions.length > 0, decisionRefinement('actions_empty')), + schema: formSchema, + uiSchema: z.record(z.string(), z.unknown()).optional(), + proposalSourceNodeId: z.string().optional(), + deadline: deadlineSchema.optional(), + }) + .superRefine((request, context) => { + const seenNames = new Set(); + const firstIndexByEffect = new Map(); + + for (const [index, action] of request.actions.entries()) { + if (seenNames.has(action.name)) { + context.addIssue(decisionIssue('duplicate_action_name', ['actions', index, 'name'], action.name)); + } + seenNames.add(action.name); + + if (firstIndexByEffect.has(action.effect)) { + context.addIssue(decisionIssue('duplicate_effect', ['actions', index, 'effect'], action.effect)); + } else { + firstIndexByEffect.set(action.effect, index); + } + } + + const resumeIndex = firstIndexByEffect.get('resume'); + if (resumeIndex === undefined) { + context.addIssue(decisionIssue('resume_required', ['actions'])); + return; + } + + const rejectIndex = firstIndexByEffect.get('reject'); + if (rejectIndex === undefined) return; + const resume = request.actions[resumeIndex]; + const reject = request.actions[rejectIndex]; + if (resume.effect === 'resume' && reject.effect === 'reject' && resume.port === reject.port) { + context.addIssue( + decisionIssue('reject_port_equals_resume_port', ['actions', rejectIndex, 'port'], reject.port), + ); + } + }), +); diff --git a/apps/backend/src/domain/decision/proposal-source.test.ts b/apps/backend/src/domain/decision/proposal-source.test.ts new file mode 100644 index 000000000..86d77abd5 --- /dev/null +++ b/apps/backend/src/domain/decision/proposal-source.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from 'vitest'; + +import type { DecisionRequest } from '@workflow-builder/types/workflow-execution/decision-request'; + +import { resolveProposalSource } from './proposal-source'; + +function request(proposalSourceNodeId?: string): DecisionRequest { + return { + version: 1, + actions: [{ name: 'approve', label: 'Approve', effect: 'resume', port: 'approved' }], + schema: { type: 'object', properties: {} }, + ...(proposalSourceNodeId === undefined ? {} : { proposalSourceNodeId }), + }; +} + +function edge(sourceNodeId: string, targetNodeId: string) { + return { sourceNodeId, targetNodeId }; +} + +describe('resolveProposalSource', () => { + it('reports a node without a request, or an unknown id, as a node without a decision request', () => { + const nodes = [{ id: 'plain' }]; + + expect(resolveProposalSource(nodes, [], 'plain')).toEqual({ error: 'node_without_decision_request' }); + expect(resolveProposalSource(nodes, [], 'missing')).toEqual({ error: 'node_without_decision_request' }); + }); + + it('returns an explicit source that is a direct predecessor', () => { + const nodes = [{ id: 'a' }, { id: 'b' }, { id: 'review', decisionRequest: request('a') }]; + const edges = [edge('a', 'review'), edge('b', 'review')]; + + expect(resolveProposalSource(nodes, edges, 'review')).toEqual({ sourceNodeId: 'a' }); + }); + + it('rejects an explicit source that is not a direct predecessor, including a successor', () => { + const nodes = [{ id: 'a' }, { id: 'after' }, { id: 'review', decisionRequest: request('after') }]; + const edges = [edge('a', 'review'), edge('review', 'after')]; + + expect(resolveProposalSource(nodes, edges, 'review')).toEqual({ error: 'explicit_source_not_a_predecessor' }); + }); + + it('falls back to the only direct predecessor when no source is declared', () => { + const nodes = [{ id: 'a' }, { id: 'review', decisionRequest: request() }]; + + expect(resolveProposalSource(nodes, [edge('a', 'review')], 'review')).toEqual({ sourceNodeId: 'a' }); + }); + + it('counts parallel edges from one node as a single predecessor', () => { + const nodes = [{ id: 'a' }, { id: 'review', decisionRequest: request() }]; + const edges = [edge('a', 'review'), edge('a', 'review')]; + + expect(resolveProposalSource(nodes, edges, 'review')).toEqual({ sourceNodeId: 'a' }); + }); + + it('does not count a self-loop as a predecessor, explicit or implicit', () => { + const explicitSelf = [{ id: 'a' }, { id: 'review', decisionRequest: request('review') }]; + const implicitSelf = [{ id: 'review', decisionRequest: request() }]; + + expect(resolveProposalSource(explicitSelf, [edge('a', 'review'), edge('review', 'review')], 'review')).toEqual({ + error: 'explicit_source_not_a_predecessor', + }); + expect(resolveProposalSource(implicitSelf, [edge('review', 'review')], 'review')).toEqual({ + error: 'no_predecessor', + }); + }); + + it('reports no predecessor when the node has only outgoing edges', () => { + const nodes = [{ id: 'review', decisionRequest: request() }, { id: 'after' }]; + + expect(resolveProposalSource(nodes, [edge('review', 'after')], 'review')).toEqual({ error: 'no_predecessor' }); + }); + + it('reports ambiguity when several predecessors exist and none is declared', () => { + const nodes = [{ id: 'a' }, { id: 'b' }, { id: 'review', decisionRequest: request() }]; + const edges = [edge('a', 'review'), edge('b', 'review')]; + + expect(resolveProposalSource(nodes, edges, 'review')).toEqual({ error: 'ambiguous_predecessor' }); + }); +}); diff --git a/apps/backend/src/domain/decision/proposal-source.ts b/apps/backend/src/domain/decision/proposal-source.ts new file mode 100644 index 000000000..755842470 --- /dev/null +++ b/apps/backend/src/domain/decision/proposal-source.ts @@ -0,0 +1,44 @@ +import { unique } from 'remeda'; + +import type { BaseNode, WorkflowEdgeDefinition } from '@workflow-builder/types/workflow-execution/execution-model'; + +type GraphNode = Pick; +type GraphEdge = Pick; + +export type UnresolvedSourceReason = + | 'node_without_decision_request' + | 'explicit_source_not_a_predecessor' + | 'no_predecessor' + | 'ambiguous_predecessor'; + +// Result shape: apps/backend/decision-request.decision-log.md, decision 9. +export type ProposalSourceResolution = + | { sourceNodeId: string; error?: undefined } + | { sourceNodeId?: undefined; error: UnresolvedSourceReason }; + +// Takes the execution-model shape so a caller holding a WorkflowDefinition passes its +// nodes and edges straight in; the snapshot schema adapts before calling. +export function resolveProposalSource( + nodes: readonly GraphNode[], + edges: readonly GraphEdge[], + nodeId: string, +): ProposalSourceResolution { + const node = nodes.find((candidate) => candidate.id === nodeId); + if (node?.decisionRequest === undefined) return { error: 'node_without_decision_request' }; + + // A self-loop is not a predecessor. + const predecessors = unique( + edges + .filter((edge) => edge.targetNodeId === nodeId && edge.sourceNodeId !== nodeId) + .map((edge) => edge.sourceNodeId), + ); + const explicit = node.decisionRequest.proposalSourceNodeId; + if (explicit !== undefined) { + return predecessors.includes(explicit) + ? { sourceNodeId: explicit } + : { error: 'explicit_source_not_a_predecessor' }; + } + if (predecessors.length === 0) return { error: 'no_predecessor' }; + if (predecessors.length > 1) return { error: 'ambiguous_predecessor' }; + return { sourceNodeId: predecessors[0] }; +} diff --git a/apps/backend/src/domain/decision/validate-submitted-decision.test.ts b/apps/backend/src/domain/decision/validate-submitted-decision.test.ts new file mode 100644 index 000000000..caa3fe817 --- /dev/null +++ b/apps/backend/src/domain/decision/validate-submitted-decision.test.ts @@ -0,0 +1,392 @@ +import { describe, expect, it } from 'vitest'; + +import type { DecisionRequest } from '@workflow-builder/types/workflow-execution/decision-request'; + +import { type SubmittedDecisionErrorCode, submittedDecisionErrorMessage } from './decision-issues'; +import { + type SubmittedDecision, + submittedDecisionSchema, + validateSubmittedDecision, +} from './validate-submitted-decision'; + +const approve = { name: 'approve', label: 'Approve', effect: 'resume', port: 'approved' } as const; +const reject = { name: 'reject', label: 'Reject', effect: 'reject', port: 'rejected', reasonRequired: false } as const; +const askAgain = { name: 'ask-again', label: 'Ask again', effect: 'rerun-source', maxIterations: 3 } as const; + +// As the parser leaves it: defaults present, every action explicit. +function requestWith(overrides: Partial = {}): DecisionRequest { + return { + version: 1, + actions: [approve, reject, askAgain], + schema: { + type: 'object', + properties: { + orderDate: { type: 'string', readOnly: true }, + customerEmail: { type: 'string', readOnly: true, 'x-pii': true }, + refundAmount: { type: 'number' }, + emailDraft: { type: 'string', readOnly: false }, + note: { type: 'string' }, + }, + required: ['refundAmount'], + }, + ...overrides, + }; +} + +// A form the SDK's own field model produces: an object with children and an array of +// objects. Editability lives on the children, not on the wrapper. +const nestedRequest = (): DecisionRequest => + requestWith({ + schema: { + type: 'object', + properties: { + profile: { + type: 'object', + required: ['nickname'], + properties: { id: { type: 'string', readOnly: true }, nickname: { type: 'string' } }, + }, + lines: { + type: 'array', + items: { + type: 'object', + properties: { + sku: { type: 'string', readOnly: true }, + qty: { type: 'number' }, + origin: { type: 'object', properties: { warehouse: { type: 'string', readOnly: true } } }, + }, + }, + }, + }, + }, + }); + +describe('validateSubmittedDecision', () => { + it.each<{ name: string; request?: DecisionRequest; call: SubmittedDecision; effect: string }>([ + { name: 'a resume without edits resumes', call: { action: 'approve' }, effect: 'resume' }, + { name: 'a resume with empty edits resumes', call: { action: 'approve', edits: {} }, effect: 'resume' }, + { + name: 'a resume with an edit on an editable field resumes with edits', + call: { action: 'approve', edits: { refundAmount: 42 } }, + effect: 'resume-with-edits', + }, + { + name: 'an explicit readOnly: false is editable', + call: { action: 'approve', edits: { emailDraft: 'Dear customer' } }, + effect: 'resume-with-edits', + }, + { + name: 'a required field set to a value is fine', + call: { action: 'approve', edits: { refundAmount: 0 } }, + effect: 'resume-with-edits', + }, + { + name: 'an optional field may be emptied', + call: { action: 'approve', edits: { note: '' } }, + effect: 'resume-with-edits', + }, + { + name: 'editable children of an object and of an array item', + request: nestedRequest(), + call: { action: 'approve', edits: { profile: { nickname: 'Ada' }, lines: [{ qty: 3 }] } }, + effect: 'resume-with-edits', + }, + { name: 'a reject without a reason when none is required', call: { action: 'reject' }, effect: 'reject' }, + { + name: 'a reject with a reason when one is required', + request: requestWith({ actions: [approve, { ...reject, reasonRequired: true }] }), + call: { action: 'reject', reason: 'Amount exceeds policy' }, + effect: 'reject', + }, + { + name: 'a rerun with a comment', + call: { action: 'ask-again', comment: 'Use the discounted price' }, + effect: 'rerun-source', + }, + ])('accepts $name', ({ request = requestWith(), call, effect }) => { + const result = validateSubmittedDecision(request, call); + + expect(result.error).toBeUndefined(); + expect(result.decision?.effect).toBe(effect); + expect(result.decision?.action).toBe(call.action); + expect(result.action?.name).toBe(call.action); + }); + + // Open point in the decision log: until decided otherwise, edits on a non-resume action + // are checked field by field, kept on the decision, and leave the effect alone. + it.each<{ call: SubmittedDecision; effect: string }>([ + { call: { action: 'reject', reason: 'late', edits: { refundAmount: 0 } }, effect: 'reject' }, + { call: { action: 'ask-again', comment: 'again', edits: { note: 'x' } }, effect: 'rerun-source' }, + ])('keeps the declared effect $effect and carries the edits of a non-resume submission', ({ call, effect }) => { + const result = validateSubmittedDecision(requestWith(), call); + + expect(result.decision?.effect).toBe(effect); + expect(result.decision?.edits).toEqual(call.edits); + }); + + it.each<{ + name: string; + request?: DecisionRequest; + call: SubmittedDecision; + code: SubmittedDecisionErrorCode; + value: string; + path: string[]; + }>([ + { + name: 'an action the request does not offer', + call: { action: 'escalate' }, + code: 'unknown_action', + value: 'escalate', + path: ['action'], + }, + { + name: "an action addressed by its label instead of its name ('Approve')", + call: { action: 'Approve' }, + code: 'unknown_action', + value: 'Approve', + path: ['action'], + }, + { + name: 'a reject on a request without a reject action', + request: requestWith({ actions: [approve] }), + call: { action: 'reject', reason: 'no' }, + code: 'unknown_action', + value: 'reject', + path: ['action'], + }, + { + name: 'a rerun on a request without a rerun-source action', + request: requestWith({ actions: [approve, reject] }), + call: { action: 'ask-again', comment: 'again' }, + code: 'unknown_action', + value: 'ask-again', + path: ['action'], + }, + { + name: 'a reject without a reason when one is required', + request: requestWith({ actions: [approve, { ...reject, reasonRequired: true }] }), + call: { action: 'reject' }, + code: 'reason_required', + value: 'reject', + path: ['reason'], + }, + { + name: 'a reject with a blank reason when one is required', + request: requestWith({ actions: [approve, { ...reject, reasonRequired: true }] }), + call: { action: 'reject', reason: ' ' }, + code: 'reason_required', + value: 'reject', + path: ['reason'], + }, + { + name: 'a rerun without a comment', + call: { action: 'ask-again' }, + code: 'comment_required', + value: 'ask-again', + path: ['comment'], + }, + { + name: 'a rerun with a whitespace-only comment', + call: { action: 'ask-again', comment: ' \n ' }, + code: 'comment_required', + value: 'ask-again', + path: ['comment'], + }, + { + name: 'an edit on a read-only field', + call: { action: 'approve', edits: { orderDate: '2026-01-01' } }, + code: 'field_not_editable', + value: 'orderDate', + path: ['edits', 'orderDate'], + }, + { + name: 'an edit on a field the schema does not declare', + call: { action: 'approve', edits: { discount: 10 } }, + code: 'unknown_field', + value: 'discount', + path: ['edits', 'discount'], + }, + { + name: 'an edit on a field that exists only on Object.prototype', + call: { action: 'approve', edits: { constructor: 1 } }, + code: 'unknown_field', + value: 'constructor', + path: ['edits', 'constructor'], + }, + { + name: 'a required field emptied with an empty string', + call: { action: 'approve', edits: { refundAmount: '' } }, + code: 'required_field_missing', + value: 'refundAmount', + path: ['edits', 'refundAmount'], + }, + { + name: 'a required field emptied with null', + call: { action: 'approve', edits: { refundAmount: null } }, + code: 'required_field_missing', + value: 'refundAmount', + path: ['edits', 'refundAmount'], + }, + { + name: 'a required field emptied with undefined', + call: { action: 'approve', edits: { refundAmount: undefined } }, + code: 'required_field_missing', + value: 'refundAmount', + path: ['edits', 'refundAmount'], + }, + { + name: 'a read-only child rewritten by replacing the object that holds it', + request: nestedRequest(), + call: { action: 'approve', edits: { profile: { id: 'changed' } } }, + code: 'field_not_editable', + value: 'id', + path: ['edits', 'profile', 'id'], + }, + { + name: "a child the object's own required list names, emptied", + request: nestedRequest(), + call: { action: 'approve', edits: { profile: { nickname: null } } }, + code: 'required_field_missing', + value: 'nickname', + path: ['edits', 'profile', 'nickname'], + }, + { + name: 'a child the nested object does not declare', + request: nestedRequest(), + call: { action: 'approve', edits: { profile: { ghost: 1 } } }, + code: 'unknown_field', + value: 'ghost', + path: ['edits', 'profile', 'ghost'], + }, + { + name: 'a read-only child of an array item, named with its index', + request: nestedRequest(), + call: { action: 'approve', edits: { lines: [{ qty: 2 }, { sku: 'swapped' }] } }, + code: 'field_not_editable', + value: 'sku', + path: ['edits', 'lines', '1', 'sku'], + }, + { + name: 'a child of an object the form declares but never describes', + request: requestWith({ schema: { type: 'object', properties: { opaque: { type: 'object' } } } }), + call: { action: 'approve', edits: { opaque: { anything: 1 } } }, + code: 'unknown_field', + value: 'anything', + path: ['edits', 'opaque', 'anything'], + }, + { + name: 'an element of an array the form declares but never describes', + request: requestWith({ schema: { type: 'object', properties: { rows: { type: 'array' } } } }), + call: { action: 'approve', edits: { rows: [{ anything: 1 }] } }, + code: 'unknown_field', + value: '0', + path: ['edits', 'rows', '0'], + }, + { + name: 'any element of an array whose items are read-only, named by its index', + request: requestWith({ + schema: { type: 'object', properties: { rows: { type: 'array', items: { readOnly: true } } } }, + }), + call: { action: 'approve', edits: { rows: ['a', 'b'] } }, + code: 'field_not_editable', + value: '0', + path: ['edits', 'rows', '0'], + }, + { + name: 'a read-only field three levels down, through an array item', + request: nestedRequest(), + call: { action: 'approve', edits: { lines: [{ qty: 1 }, { origin: { warehouse: 'moved' } }] } }, + code: 'field_not_editable', + value: 'warehouse', + path: ['edits', 'lines', '1', 'origin', 'warehouse'], + }, + { + name: 'a nested child that exists only on Object.prototype', + request: nestedRequest(), + call: { action: 'approve', edits: { profile: { constructor: 1 } } }, + code: 'unknown_field', + value: 'constructor', + path: ['edits', 'profile', 'constructor'], + }, + ])('refuses $name', ({ request = requestWith(), call, code, value, path }) => { + expect(validateSubmittedDecision(request, call)).toEqual({ + error: { code, message: submittedDecisionErrorMessage(code, value), path }, + }); + }); + + // The decision log promises the first refusal, not a list. Submission order decides which. + it('reports only the first bad edit, in the order they were submitted', () => { + const readOnlyFirst = validateSubmittedDecision(requestWith(), { + action: 'approve', + edits: { orderDate: '2026-01-01', discount: 10 }, + }); + const unknownFirst = validateSubmittedDecision(requestWith(), { + action: 'approve', + edits: { discount: 10, orderDate: '2026-01-01' }, + }); + + expect(readOnlyFirst.error).toMatchObject({ code: 'field_not_editable', path: ['edits', 'orderDate'] }); + expect(unknownFirst.error).toMatchObject({ code: 'unknown_field', path: ['edits', 'discount'] }); + }); + + it('records the action by name and returns the matched action beside the decision', () => { + const submitted = { action: 'approve', edits: { refundAmount: 12 }, comment: 'rounded down' }; + + const result = validateSubmittedDecision(requestWith(), submitted); + + expect(result.decision).toEqual({ + action: 'approve', + effect: 'resume-with-edits', + edits: { refundAmount: 12 }, + comment: 'rounded down', + }); + expect(result.action).toEqual(approve); + }); + + it('defaults edits to an empty object when none were submitted', () => { + expect(validateSubmittedDecision(requestWith(), { action: 'reject', reason: 'late' }).decision).toEqual({ + action: 'reject', + effect: 'reject', + edits: {}, + reason: 'late', + }); + }); +}); + +describe('submittedDecisionSchema', () => { + it('accepts a full submission and strips keys it does not know', () => { + const parsed = submittedDecisionSchema.parse({ + action: 'approve', + edits: { a: 1 }, + reason: 'r', + comment: 'c', + extra: true, + }); + + expect(parsed).toEqual({ action: 'approve', edits: { a: 1 }, reason: 'r', comment: 'c' }); + }); + + it('drops an own __proto__ key in edits instead of making it the prototype', () => { + const parsed = submittedDecisionSchema.parse( + JSON.parse('{"action":"approve","edits":{"__proto__":{"refundAmount":1}}}'), + ); + + expect(parsed.edits).toEqual({}); + expect(Object.getPrototypeOf(parsed.edits)).toBe(Object.prototype); + }); + + it.each<{ name: string; body: unknown; path: string }>([ + { name: 'edits as an array', body: { action: 'approve', edits: [] }, path: 'edits' }, + { name: 'edits as a number', body: { action: 'approve', edits: 42 }, path: 'edits' }, + { name: 'edits as null', body: { action: 'approve', edits: null }, path: 'edits' }, + { name: 'edits as a string', body: { action: 'approve', edits: 'x' }, path: 'edits' }, + { name: 'a non-string action', body: { action: 42 }, path: 'action' }, + { name: 'a non-string reason', body: { action: 'reject', reason: 42 }, path: 'reason' }, + { name: 'a non-string comment', body: { action: 'ask-again', comment: {} }, path: 'comment' }, + { name: 'a missing action', body: { edits: {} }, path: 'action' }, + ])('rejects $name before the rules ever run', ({ body, path }) => { + const result = submittedDecisionSchema.safeParse(body); + + expect(result.success).toBe(false); + expect(result.success ? [] : result.error.issues.map((issue) => issue.path.join('.'))).toEqual([path]); + }); +}); diff --git a/apps/backend/src/domain/decision/validate-submitted-decision.ts b/apps/backend/src/domain/decision/validate-submitted-decision.ts new file mode 100644 index 000000000..0000651f0 --- /dev/null +++ b/apps/backend/src/domain/decision/validate-submitted-decision.ts @@ -0,0 +1,133 @@ +import { z } from 'zod'; + +import type { + Decision, + DecisionAction, + DecisionEffect, + DecisionRequest, +} from '@workflow-builder/types/workflow-execution/decision-request'; + +import { type SubmittedDecisionErrorCode, submittedDecisionErrorMessage } from './decision-issues'; + +// The shape of what the decider sent. The caller parses a body with this before calling +// `validateSubmittedDecision`, which assumes the shape and checks only the rules. +// Provisional: the decision endpoint owns the public request shape and may rename fields. +export const submittedDecisionSchema = z.object({ + action: z.string(), + edits: z.record(z.string(), z.unknown()).optional(), + reason: z.string().optional(), + comment: z.string().optional(), +}); + +export type SubmittedDecision = z.infer; + +export type SubmittedDecisionError = { code: SubmittedDecisionErrorCode; message: string; path?: string[] }; + +// `action` is the matched action, for routing; the decision itself records only its name. +export type SubmittedDecisionResult = + | { decision: Decision; action: DecisionAction; error?: undefined } + | { decision?: undefined; action?: undefined; error: SubmittedDecisionError }; + +function refuse(code: SubmittedDecisionErrorCode, value: string, path: string[]): SubmittedDecisionResult { + return { error: { code, message: submittedDecisionErrorMessage(code, value), path } }; +} + +function isBlank(text: string | undefined): boolean { + return text === undefined || text.trim().length === 0; +} + +// "Emptied" means the decider cleared the field, not that they typed something invalid. +function isEmptied(value: unknown): boolean { + return value === undefined || value === null || (typeof value === 'string' && value.trim().length === 0); +} + +// The request arrived through the parser, so `schema` has the shape checked there; the +// reads below only narrow what `Record` hides. +function asObject(value: unknown): Record | undefined { + return typeof value === 'object' && value !== null && !Array.isArray(value) + ? (value as Record) + : undefined; +} + +type EditedChild = { key: string; value: unknown; declared: Record | undefined }; + +// What an edited value's children are and which schema declares each. Undefined for a leaf, +// which has none. An array's elements are all declared by `items`, so an index carries no +// rules of its own; a level the form does not describe inline declares nothing at all. +function childrenOf( + schema: Record, + edited: unknown, +): { children: EditedChild[]; required: Set } | undefined { + const fields = asObject(edited); + if (fields !== undefined) { + const properties = asObject(schema['properties']) ?? {}; + const names = schema['required']; + return { + children: Object.entries(fields).map(([key, value]) => ({ + key, + value, + declared: Object.hasOwn(properties, key) ? (asObject(properties[key]) ?? {}) : undefined, + })), + required: new Set(Array.isArray(names) ? (names as string[]) : []), + }; + } + + if (!Array.isArray(edited)) return undefined; + const items = asObject(schema['items']); + return { + children: edited.map((value, index) => ({ key: `${index}`, value, declared: items })), + required: new Set(), + }; +} + +// Every level the form declares, not just the outermost one: a `readOnly` child would +// otherwise be rewritten by replacing the object that holds it +// (follow-up: decision-edit-schema-composition). +function validateEdits( + schema: Record, + edited: unknown, + path: string[], +): SubmittedDecisionResult | undefined { + const level = childrenOf(schema, edited); + if (level === undefined) return undefined; + + for (const { key, value, declared } of level.children) { + const here = [...path, key]; + if (declared === undefined) return refuse('unknown_field', key, here); + if (declared['readOnly'] === true) return refuse('field_not_editable', key, here); + if (level.required.has(key) && isEmptied(value)) return refuse('required_field_missing', key, here); + + const refused = validateEdits(declared, value, here); + if (refused !== undefined) return refused; + } + + return undefined; +} + +// Presence and editability only. Whether an edited value fits its declared type is a +// later concern with its own validator (follow-up: decision-edit-value-validation) +export function validateSubmittedDecision( + request: DecisionRequest, + submitted: SubmittedDecision, +): SubmittedDecisionResult { + const action = request.actions.find((candidate) => candidate.name === submitted.action); + if (action === undefined) return refuse('unknown_action', submitted.action, ['action']); + + if (action.effect === 'reject' && action.reasonRequired && isBlank(submitted.reason)) { + return refuse('reason_required', action.name, ['reason']); + } + if (action.effect === 'rerun-source' && isBlank(submitted.comment)) { + return refuse('comment_required', action.name, ['comment']); + } + + const edits = submitted.edits ?? {}; + const refused = validateEdits(request.schema, edits, ['edits']); + if (refused !== undefined) return refused; + + const withEdits = Object.keys(edits).length > 0; + const effect: DecisionEffect = action.effect === 'resume' && withEdits ? 'resume-with-edits' : action.effect; + return { + decision: { action: action.name, effect, edits, reason: submitted.reason, comment: submitted.comment }, + action, + }; +} diff --git a/apps/backend/src/domain/mapper/from-integration-data.ts b/apps/backend/src/domain/mapper/from-integration-data.ts index 803ff5de5..5a20969f8 100644 --- a/apps/backend/src/domain/mapper/from-integration-data.ts +++ b/apps/backend/src/domain/mapper/from-integration-data.ts @@ -28,11 +28,14 @@ export function mapToExecutionModel(workflowId: string, data: WorkflowSnapshot): return { workflowId, nodes, edges }; } -// Lifts the three fields an engine reads out of `data.properties`, where the SDK's -// `sharedProperties` put them. `role` comes from `data.isStartNode` instead, which sits -// beside the properties. `description` stays in `config`: no engine reads it. +// Lifts what an engine reads out of `data.properties`: `label`, `errorPolicy` (from the SDK's +// `sharedProperties`) and `decisionRequest` (validated and defaulted by the parse, unchecked here). +// `role` comes from `data.isStartNode` beside the properties; `description` stays in `config`. function mapNode(node: FrontendNode): BaseNode { - const { errorPolicy: rawErrorPolicy, label: rawLabel, ...config } = node.data.properties ?? {}; + // Spread first, so only own keys are read. The parse keeps unknown keys, and an own + // `__proto__` among them would leave the properties inheriting fields no schema saw; + // this is the one read that would turn such a field into a real one on the way out. + const { errorPolicy: rawErrorPolicy, label: rawLabel, decisionRequest, ...config } = { ...node.data.properties }; const errorPolicy = isErrorPolicy(rawErrorPolicy) ? rawErrorPolicy : undefined; const label = isNonEmptyString(rawLabel) ? rawLabel.trim() : undefined; const role: NodeRole | undefined = node.data.isStartNode === true ? 'start' : undefined; @@ -40,7 +43,7 @@ function mapNode(node: FrontendNode): BaseNode { id: node.id, type: node.data.type, config, - ...pickBy({ label, errorPolicy, role }, isDefined), + ...pickBy({ label, errorPolicy, decisionRequest, role }, isDefined), }; } diff --git a/apps/backend/src/domain/mapper/snapshot-schema.test.ts b/apps/backend/src/domain/mapper/snapshot-schema.test.ts index e0841f3c5..9599f12d0 100644 --- a/apps/backend/src/domain/mapper/snapshot-schema.test.ts +++ b/apps/backend/src/domain/mapper/snapshot-schema.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; +import { type DecisionIssueCode, decisionIssueMessage } from '../decision/decision-issues'; import { mapToExecutionModel } from './from-integration-data'; import { workflowSnapshotSchema } from './snapshot-schema'; @@ -115,6 +116,237 @@ describe('workflowSnapshotSchema', () => { }); }); +function node(id: string, properties?: Record) { + return { id, data: { type: 'product/any', properties } }; +} + +function edge(source: string, target: string, sourceHandle?: string) { + return { id: `${source}->${target}${sourceHandle ?? ''}`, source, target, sourceHandle }; +} + +function issuesOf(snapshot: unknown): { path: string; message: string }[] { + const result = workflowSnapshotSchema.safeParse(snapshot); + return result.success + ? [] + : result.error.issues.map((issue) => ({ path: issue.path.join('.'), message: issue.message })); +} + +function issuePaths(snapshot: unknown): string[] { + return issuesOf(snapshot).map((issue) => issue.path); +} + +describe('workflowSnapshotSchema: decision requests', () => { + const approve = { name: 'approve', label: 'Approve', effect: 'resume' }; + const askAgain = { name: 'ask-again', label: 'Ask again', effect: 'rerun-source' }; + const emptyForm = { type: 'object', properties: {} }; + + function decisionNode(id: string, decisionRequest: Record) { + return { + id, + data: { + type: 'product/any', + properties: { decisionRequest: { version: 1, schema: emptyForm, ...decisionRequest } }, + }, + }; + } + + it('parses a decision request and materialises its defaults inside properties', () => { + const parsed = workflowSnapshotSchema.parse({ + nodes: [node('src'), decisionNode('review', { actions: [approve, askAgain] })], + edges: [edge('src', 'review')], + }); + + expect(parsed.nodes[1]!.data.properties?.decisionRequest?.actions).toEqual([ + { ...approve, port: 'approved' }, + { ...askAgain, maxIterations: 3 }, + ]); + }); + + it('leaves the properties of a node without a request untouched', () => { + const properties = { label: 'Plain', decisionBranches: [{ x: 1 }], meta: { deep: { nested: true } } }; + + const parsed = workflowSnapshotSchema.parse({ nodes: [node('n1', properties)], edges: [] }); + + expect(parsed.nodes[0]!.data.properties).toEqual(properties); + }); + + it('points a request issue at the node index and field', () => { + const snapshot = { + nodes: [node('src'), decisionNode('review', { actions: [approve, { ...approve, name: 'approve-2' }] })], + edges: [edge('src', 'review')], + }; + + expect(issuePaths(snapshot)).toContain('nodes.1.data.properties.decisionRequest.actions.1.effect'); + }); + + it('rejects `decisionRequest: null`; absent is the only way to carry no request', () => { + const snapshot = { nodes: [node('n1', { decisionRequest: null })], edges: [] }; + + expect(issuePaths(snapshot)).toContain('nodes.0.data.properties.decisionRequest'); + }); + + it.each<{ name: string; snapshot: unknown }>([ + { + name: 'an explicit source that is a direct predecessor', + snapshot: { + nodes: [node('a'), node('b'), decisionNode('review', { actions: [approve], proposalSourceNodeId: 'a' })], + edges: [edge('a', 'review'), edge('b', 'review')], + }, + }, + { + name: 'a rerun-source node with exactly one predecessor and no explicit source', + snapshot: { + nodes: [node('a'), decisionNode('review', { actions: [approve, askAgain] })], + edges: [edge('a', 'review')], + }, + }, + { + name: 'a rerun-source node with several predecessors when the explicit source picks one', + snapshot: { + nodes: [ + node('a'), + node('b'), + decisionNode('review', { actions: [approve, askAgain], proposalSourceNodeId: 'b' }), + ], + edges: [edge('a', 'review'), edge('b', 'review')], + }, + }, + { + name: 'a rerun-source node whose single predecessor connects through two handles', + snapshot: { + nodes: [node('a'), decisionNode('review', { actions: [approve, askAgain] })], + edges: [edge('a', 'review', 'left'), edge('a', 'review', 'right')], + }, + }, + { + name: 'a node without rerun-source whose explicit source carries its own decision request', + snapshot: { + nodes: [ + node('a'), + decisionNode('first', { actions: [approve] }), + decisionNode('second', { actions: [approve], proposalSourceNodeId: 'first' }), + ], + edges: [edge('a', 'first'), edge('first', 'second')], + }, + }, + { + name: 'two independent deciding nodes in one snapshot', + snapshot: { + nodes: [ + node('a'), + decisionNode('review-1', { actions: [approve, askAgain] }), + node('b'), + decisionNode('review-2', { actions: [approve, askAgain] }), + ], + edges: [edge('a', 'review-1'), edge('review-1', 'b'), edge('b', 'review-2')], + }, + }, + ])('accepts $name', ({ snapshot }) => { + expect(workflowSnapshotSchema.safeParse(snapshot).success).toBe(true); + }); + + it.each<{ name: string; snapshot: unknown; path: string; issue: { code: DecisionIssueCode; value?: string } }>([ + { + name: 'an explicit source with no edge into the deciding node', + snapshot: { + nodes: [node('a'), node('b'), decisionNode('review', { actions: [approve], proposalSourceNodeId: 'b' })], + edges: [edge('a', 'review')], + }, + path: 'nodes.2.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_not_a_predecessor', value: 'b' }, + }, + { + name: 'an explicit source that is a successor, not a predecessor', + snapshot: { + nodes: [ + node('a'), + decisionNode('review', { actions: [approve], proposalSourceNodeId: 'after' }), + node('after'), + ], + edges: [edge('a', 'review'), edge('review', 'after')], + }, + path: 'nodes.1.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_not_a_predecessor', value: 'after' }, + }, + { + name: 'a rerun-source node with no predecessor', + snapshot: { + nodes: [decisionNode('review', { actions: [approve, askAgain] }), node('after')], + edges: [edge('review', 'after')], + }, + path: 'nodes.0.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_missing' }, + }, + { + name: 'a rerun-source node with several predecessors and no explicit source', + snapshot: { + nodes: [node('a'), node('b'), decisionNode('review', { actions: [approve, askAgain] })], + edges: [edge('a', 'review'), edge('b', 'review')], + }, + path: 'nodes.2.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_ambiguous' }, + }, + { + name: 'a rerun-source node whose implicit source carries its own decision request', + snapshot: { + nodes: [ + node('a'), + decisionNode('first', { actions: [approve] }), + decisionNode('second', { actions: [approve, askAgain] }), + ], + edges: [edge('a', 'first'), edge('first', 'second')], + }, + path: 'nodes.2.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_has_decision_request', value: 'first' }, + }, + { + name: 'a node with no predecessor, so its proposal source cannot be resolved', + snapshot: { nodes: [decisionNode('review', { actions: [approve] })], edges: [] }, + path: 'nodes.0.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_missing' }, + }, + { + name: 'a node with several predecessors and no explicit source', + snapshot: { + nodes: [node('a'), node('b'), decisionNode('review', { actions: [approve] })], + edges: [edge('a', 'review'), edge('b', 'review')], + }, + path: 'nodes.2.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_ambiguous' }, + }, + { + name: 'a rerun-source node whose explicit source carries its own decision request', + snapshot: { + nodes: [ + node('a'), + decisionNode('first', { actions: [approve] }), + decisionNode('second', { actions: [approve, askAgain], proposalSourceNodeId: 'first' }), + ], + edges: [edge('a', 'first'), edge('a', 'second'), edge('first', 'second')], + }, + path: 'nodes.2.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_has_decision_request', value: 'first' }, + }, + { + name: 'only the broken node when another deciding node in the snapshot is fine', + snapshot: { + nodes: [ + node('a'), + decisionNode('review-1', { actions: [approve, askAgain] }), + decisionNode('review-2', { actions: [approve, askAgain] }), + ], + edges: [edge('a', 'review-1'), edge('a', 'review-2'), edge('review-1', 'review-2')], + }, + path: 'nodes.2.data.properties.decisionRequest.proposalSourceNodeId', + issue: { code: 'source_ambiguous' }, + }, + ])('rejects $name', ({ snapshot, path, issue }) => { + const sourceIssues = issuesOf(snapshot).filter((candidate) => candidate.path.endsWith('proposalSourceNodeId')); + + expect(sourceIssues).toEqual([{ path, message: decisionIssueMessage(issue.code, issue.value) }]); + }); +}); + describe('mapToExecutionModel', () => { it('copies every property the runner does not lift into `config`', () => { const snapshot = workflowSnapshotSchema.parse({ @@ -143,6 +375,24 @@ describe('mapToExecutionModel', () => { ]); }); + // `workflowSnapshotSchema` refuses an own `__proto__`, so this reaches the mapper only if + // that guard is bypassed. The mapper is exported, so it can be. + it('ignores properties that are only inherited, however they got there', () => { + const snapshot = workflowSnapshotSchema.parse({ + nodes: [{ id: 'n1', data: { type: 'product/foo', properties: { own: 1 } } }], + edges: [], + }); + Object.setPrototypeOf(snapshot.nodes[0]?.data.properties ?? {}, { + decisionRequest: { version: 99, actions: [] }, + label: 'Inherited', + errorPolicy: 'continue', + }); + + const result = mapToExecutionModel('wf-1', snapshot); + + expect(result.nodes).toEqual([{ id: 'n1', type: 'product/foo', config: { own: 1 } }]); + }); + it('defaults `config` to `{}` when properties are absent', () => { const snapshot = workflowSnapshotSchema.parse({ nodes: [{ id: 'n1', data: { type: 'product/empty' } }], @@ -297,4 +547,79 @@ describe('mapToExecutionModel', () => { expect(result.nodes[0]?.type).toBe('never-seen-before/v3'); }); + + it('lifts a validated decision request out of `config` onto `decisionRequest`', () => { + const request = { + version: 1, + actions: [{ name: 'approve', label: 'Approve', effect: 'resume' }], + schema: { type: 'object', properties: {} }, + }; + const snapshot = workflowSnapshotSchema.parse({ + nodes: [ + { id: 'a', data: { type: 'product/any' } }, + { + id: 'review', + data: { type: 'product/any', properties: { label: 'Review', foo: 1, decisionRequest: request } }, + }, + ], + edges: [{ id: 'e1', source: 'a', target: 'review' }], + }); + + const result = mapToExecutionModel('wf-1', snapshot); + + expect(result.nodes[1]!.decisionRequest).toEqual({ + ...request, + actions: [{ ...request.actions[0], port: 'approved' }], + }); + expect(result.nodes[1]!.config).toEqual({ foo: 1 }); + expect(result.nodes[1]!.label).toBe('Review'); + }); + + it('gives a node without a request no `decisionRequest` key', () => { + const snapshot = workflowSnapshotSchema.parse({ + nodes: [ + { id: 'n1', data: { type: 'product/any', properties: { foo: 1 } } }, + { id: 'n2', data: { type: 'product/any' } }, + ], + edges: [], + }); + + const result = mapToExecutionModel('wf-1', snapshot); + + expect(result.nodes[0]).not.toHaveProperty('decisionRequest'); + expect(result.nodes[1]).not.toHaveProperty('decisionRequest'); + }); +}); + +function snapshotJson(properties: string) { + return JSON.parse(`{"nodes":[{"id":"n1","data":{"type":"product/any","properties":${properties}}}],"edges":[]}`); +} + +describe('workflowSnapshotSchema: own __proto__ keys', () => { + it('rejects a well-shaped decision request smuggled through properties.__proto__', () => { + const smuggled = snapshotJson( + '{"label":"ok","__proto__":{"decisionRequest":{"version":99,"actions":[{"effect":"bogus"}],"schema":"x"}}}', + ); + + expect(issuesOf(smuggled)).toEqual([ + { path: 'nodes.0.data.properties.__proto__', message: "the key '__proto__' is not allowed" }, + ]); + }); + + it('rejects one inside a decision request instead of inheriting the deadline it smuggles', () => { + const poisoned = snapshotJson( + '{"decisionRequest":{"version":1,"actions":[{"name":"approve","label":"Approve","effect":"resume"}],' + + '"schema":{"type":"object","properties":{}},' + + '"__proto__":{"deadline":{"after":"garbage","policy":"nuke"},"uiSchema":"x"}}}', + ); + + expect(issuePaths(poisoned)).toEqual(['nodes.0.data.properties.decisionRequest.__proto__']); + }); + + it('answers with an issue, not a throw, when the smuggled request has no actions array', () => { + const smuggled = snapshotJson('{"__proto__":{"decisionRequest":{"actions":"x"}}}'); + + expect(() => workflowSnapshotSchema.safeParse(smuggled)).not.toThrow(); + expect(issuePaths(smuggled)).toEqual(['nodes.0.data.properties.__proto__']); + }); }); diff --git a/apps/backend/src/domain/mapper/snapshot-schema.ts b/apps/backend/src/domain/mapper/snapshot-schema.ts index a7acf610b..aa517df3a 100644 --- a/apps/backend/src/domain/mapper/snapshot-schema.ts +++ b/apps/backend/src/domain/mapper/snapshot-schema.ts @@ -1,11 +1,13 @@ -// Validates the workflow snapshot at the HTTP boundary structurally only: -// every node has `id` and `data.type`; every edge has `id`, `source`, `target`. -// `data.properties` is opaque here — the backend does not know any product's -// node vocabulary. Per-type validation belongs to whichever worker registers -// executors for that vocabulary; an unknown node type surfaces at runtime as -// a `node_failed` event with the missing-executor message. +// Structural validation of the editor snapshot at the HTTP boundary. `data.properties` is +// opaque except for the reserved `decisionRequest` key: the backend knows no product's node +// vocabulary, so an unknown node type fails at runtime as `node_failed`, not here. import { z } from 'zod'; +import { type DecisionIssueCode, decisionIssue } from '../decision/decision-issues'; +import { decisionRequestSchema } from '../decision/decision-request-schema'; +import { type UnresolvedSourceReason, resolveProposalSource } from '../decision/proposal-source'; +import { rejectingOwnProtoKey } from '../schema/own-proto-key'; + const frontendNodeSchema = z.object({ id: z.string(), data: z.object({ @@ -15,7 +17,7 @@ const frontendNodeSchema = z.object({ // starts from. The editor's node kind (`start-node`, `node`, ...) is a // rendering detail and deliberately not read here. isStartNode: z.boolean().optional(), - properties: z.record(z.string(), z.unknown()).optional(), + properties: z.looseObject({ decisionRequest: decisionRequestSchema.optional() }).optional(), }), }); @@ -26,9 +28,45 @@ const frontendEdgeSchema = z.object({ sourceHandle: z.string().nullable().optional(), }); -export const workflowSnapshotSchema = z.object({ - nodes: z.array(frontendNodeSchema), - edges: z.array(frontendEdgeSchema), -}); +const SOURCE_ISSUE_BY_REASON = { + node_without_decision_request: 'source_node_without_decision_request', + explicit_source_not_a_predecessor: 'source_not_a_predecessor', + no_predecessor: 'source_missing', + ambiguous_predecessor: 'source_ambiguous', +} as const satisfies Record; + +export const workflowSnapshotSchema = rejectingOwnProtoKey( + z + .object({ + nodes: z.array(frontendNodeSchema), + edges: z.array(frontendEdgeSchema), + }) + .superRefine((snapshot, context) => { + const nodes = snapshot.nodes.map((node) => ({ + id: node.id, + decisionRequest: node.data.properties?.decisionRequest, + })); + const edges = snapshot.edges.map((edge) => ({ sourceNodeId: edge.source, targetNodeId: edge.target })); + + for (const [index, node] of snapshot.nodes.entries()) { + const request = node.data.properties?.decisionRequest; + if (request === undefined) continue; + const declaresRerun = request.actions.some((action) => action.effect === 'rerun-source'); + + const path = ['nodes', index, 'data', 'properties', 'decisionRequest', 'proposalSourceNodeId']; + const resolution = resolveProposalSource(nodes, edges, node.id); + if (resolution.error !== undefined) { + context.addIssue(decisionIssue(SOURCE_ISSUE_BY_REASON[resolution.error], path, request.proposalSourceNodeId)); + continue; + } + const sourceHasRequest = nodes.some( + (other) => other.id === resolution.sourceNodeId && other.decisionRequest !== undefined, + ); + if (declaresRerun && sourceHasRequest) { + context.addIssue(decisionIssue('source_has_decision_request', path, resolution.sourceNodeId)); + } + } + }), +); export type WorkflowSnapshot = z.infer; diff --git a/apps/backend/src/domain/schema/own-proto-key.test.ts b/apps/backend/src/domain/schema/own-proto-key.test.ts new file mode 100644 index 000000000..c34077cf6 --- /dev/null +++ b/apps/backend/src/domain/schema/own-proto-key.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; + +import { findOwnProtoKey, rejectingOwnProtoKey } from './own-proto-key'; + +describe('findOwnProtoKey', () => { + it('reports the path of an own key created by JSON.parse, through objects and arrays', () => { + expect(findOwnProtoKey({ a: { b: [1, { c: null }] } })).toBeUndefined(); + expect(findOwnProtoKey(JSON.parse('{"a": {"b": {"__proto__": {}}}}'))).toEqual(['a', 'b', '__proto__']); + expect(findOwnProtoKey(JSON.parse('{"items": [1, {"__proto__": {}}]}'))).toEqual(['items', 1, '__proto__']); + }); + + it('terminates on a cyclic object', () => { + const cyclic: Record = { a: 1 }; + cyclic['self'] = cyclic; + + expect(findOwnProtoKey(cyclic)).toBeUndefined(); + }); + + // Depth is the client's to pick and only the 1 MB body limit caps it: 50k levels of + // `[` is 100 KB, and a recursive walk died long before that. + it('walks a snapshot nested far deeper than a recursive scan could', () => { + const depth = 50_000; + const nested = (leaf: string): unknown => JSON.parse('['.repeat(depth) + leaf + ']'.repeat(depth)); + + expect(findOwnProtoKey(nested(''))).toBeUndefined(); + + const found = findOwnProtoKey(nested('{"__proto__":{}}')); + + expect(found?.at(-1)).toBe('__proto__'); + expect(found).toHaveLength(depth + 1); + }); +}); + +describe('rejectingOwnProtoKey', () => { + const schema = rejectingOwnProtoKey(z.looseObject({ a: z.number(), deadline: z.string().optional() })); + + it('passes a clean value through and rejects an own __proto__ key at its path', () => { + expect(schema.parse({ a: 1, extra: true })).toEqual({ a: 1, extra: true }); + + const result = schema.safeParse(JSON.parse('{"a": 1, "__proto__": {"deadline": "smuggled"}}')); + + expect(result.success).toBe(false); + expect(result.success ? [] : result.error.issues.map((issue) => [issue.path.join('.'), issue.message])).toEqual([ + ['__proto__', "the key '__proto__' is not allowed"], + ]); + }); +}); diff --git a/apps/backend/src/domain/schema/own-proto-key.ts b/apps/backend/src/domain/schema/own-proto-key.ts new file mode 100644 index 000000000..7f929dd04 --- /dev/null +++ b/apps/backend/src/domain/schema/own-proto-key.ts @@ -0,0 +1,59 @@ +import { z } from 'zod'; + +const OWN_PROTO_KEY_MESSAGE = "the key '__proto__' is not allowed"; + +function isWalkable(value: unknown): value is object { + return typeof value === 'object' && value !== null; +} + +// Arrays yield numeric indices, objects string keys, which is the shape a JSON path takes. +function ownEntries(value: object): Iterator<[PropertyKey, unknown]> { + return Array.isArray(value) ? value.entries() : Object.entries(value).values(); +} + +// `JSON.parse` turns "__proto__" into an ordinary own key. zod's loose objects copy unknown +// keys with a plain assignment, which for that key swaps the output's prototype instead, +// so anything under it is read back as if validated. Refuse it before parsing. +// +// Walked with an explicit stack, never recursion: a snapshot's nesting depth is whatever the +// client sent, and a blown call stack would answer 500 where this promises a 400. +export function findOwnProtoKey(value: unknown): PropertyKey[] | undefined { + if (!isWalkable(value)) return undefined; + if (Object.hasOwn(value, '__proto__')) return ['__proto__']; + + const seen = new Set([value]); + // One segment per stacked iterator below the root, so the path is copied once, on a hit. + const path: PropertyKey[] = []; + const stack: Iterator<[PropertyKey, unknown]>[] = [ownEntries(value)]; + + while (stack.length > 0) { + const step = stack.at(-1)!.next(); + if (step.done === true) { + stack.pop(); + // A no-op for the root, which owns no segment. + path.pop(); + continue; + } + + const [key, child] = step.value; + if (!isWalkable(child) || seen.has(child)) continue; + if (Object.hasOwn(child, '__proto__')) return [...path, key, '__proto__']; + + seen.add(child); + path.push(key); + stack.push(ownEntries(child)); + } + + return undefined; +} + +// Refuses the key outright, at its path. Wrap a payload where it enters: every loose +// object below is then safe without each one having to defend itself. +export function rejectingOwnProtoKey(schema: T) { + return z.preprocess((value, context) => { + const path = findOwnProtoKey(value); + if (path === undefined) return value; + context.addIssue({ code: 'custom', message: OWN_PROTO_KEY_MESSAGE, path }); + return z.NEVER; + }, schema); +} diff --git a/apps/backend/src/routes/snapshot-validation.ts b/apps/backend/src/routes/snapshot-validation.ts new file mode 100644 index 000000000..242c7ca12 --- /dev/null +++ b/apps/backend/src/routes/snapshot-validation.ts @@ -0,0 +1,40 @@ +import type { Context } from 'hono'; +import { z } from 'zod'; + +import type { SourceVersion } from '@workflow-builder/types/workflow-execution/api'; + +import { decisionIssueOf } from '../domain/decision/decision-issues'; +import { type WorkflowSnapshot, workflowSnapshotSchema } from '../domain/mapper/snapshot-schema'; +import { logger as backendLogger } from '../logger'; + +const logger = backendLogger.child({ component: 'snapshot-validation' }); + +// `code` is zod's; a domain issue adds `domainCode` and `params` so a client never keys on `message`. +export function formatValidationDetails(error: z.ZodError) { + return error.issues.map((issue) => { + const detail = { path: issue.path, message: issue.message, code: issue.code }; + const domain = decisionIssueOf(issue); + if (domain === undefined) return detail; + return { ...detail, domainCode: domain.issue, params: domain.value === undefined ? {} : { value: domain.value } }; + }); +} + +export type SnapshotParse = + | { snapshot: WorkflowSnapshot; response?: undefined } + | { snapshot?: undefined; response: Response }; + +// Publish and execute reject a snapshot the same way, so the two never drift. +export function parseSnapshot( + c: Context, + snapshotJson: unknown, + source: { workflowId: string; sourceVersion: SourceVersion }, +): SnapshotParse { + const parsed = z.safeParse(workflowSnapshotSchema, snapshotJson); + if (parsed.success) return { snapshot: parsed.data }; + + const details = formatValidationDetails(parsed.error); + logger.warn('snapshot invalid', { ...source, error: { issues: details } }); + return { + response: c.json({ code: 'invalid_snapshot', message: 'Workflow snapshot failed validation', details }, 400), + }; +} diff --git a/apps/backend/src/routes/workflows.test.ts b/apps/backend/src/routes/workflows.test.ts index 5e6bacb54..1b7048953 100644 --- a/apps/backend/src/routes/workflows.test.ts +++ b/apps/backend/src/routes/workflows.test.ts @@ -268,3 +268,222 @@ describe('createWorkflowsRoutes - execute propagates tenant identity', () => { expect(engineMock.submit).toHaveBeenCalledWith(expect.objectContaining({ variables: {} })); }); }); + +// ---- snapshot validation on publish and execute ----------------------------- +// Never on draft save: a draft is legitimately mid-edit. + +function snapshotWithDecisionActions(actions: unknown[], properties: Record = {}) { + return { + nodes: [ + { id: 'src', data: { type: 'product/any' } }, + { + id: 'review', + data: { + type: 'product/any', + properties: { decisionRequest: { version: 1, actions, schema: { type: 'object', properties } } }, + }, + }, + ], + edges: [{ id: 'e1', source: 'src', target: 'review' }], + }; +} + +const approve = { name: 'approve', label: 'Approve', effect: 'resume' }; +const validDecisionSnapshot = snapshotWithDecisionActions([approve]); +const twoResumesSnapshot = snapshotWithDecisionActions([approve, { ...approve, name: 'approve-2' }]); + +type InvalidSnapshotBody = { + code: string; + details: { path: (string | number)[]; code?: string; domainCode?: string; params?: Record }[]; +}; + +function allowAllApp() { + return buildApp(allowAll(vi.fn(async () => true))); +} + +function publish(app: ReturnType) { + return app.request('/api/workflows/w-1/publish', { method: 'POST' }); +} + +function jsonRequest(app: ReturnType, path: string, method: string, body: unknown) { + return app.request(path, { method, body: JSON.stringify(body), headers: { 'content-type': 'application/json' } }); +} + +describe('createWorkflowsRoutes - snapshot validation on publish', () => { + it('rejects a draft with a broken decision request and writes nothing', async () => { + databaseMock.select.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: twoResumesSnapshot }])); + + const response = await publish(allowAllApp()); + const body = (await response.json()) as InvalidSnapshotBody; + + expect(response.status).toBe(400); + expect(body.code).toBe('invalid_snapshot'); + const detail = body.details.find( + (candidate) => candidate.path.join('.') === 'nodes.1.data.properties.decisionRequest.actions.1.effect', + ); + // Beside zod's `code` and the English message, the identifier a client keys on and its value. + expect(detail).toMatchObject({ code: 'custom', domainCode: 'duplicate_effect', params: { value: 'resume' } }); + expect(databaseMock.update).not.toHaveBeenCalled(); + }); + + it('accepts a draft with a valid decision request and returns the row', async () => { + const published = { ...fakeWorkflow, draftJson: validDecisionSnapshot, publishedJson: validDecisionSnapshot }; + databaseMock.select.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: validDecisionSnapshot }])); + databaseMock.update.mockReturnValue(chainResolving([published])); + + const response = await publish(allowAllApp()); + + expect(response.status).toBe(200); + expect(databaseMock.update).toHaveBeenCalledTimes(1); + expect(await response.json()).toMatchObject({ id: 'w-1', publishedJson: validDecisionSnapshot }); + }); + + it('accepts a form whose properties carry no type, as JSON Schema allows', async () => { + const draftJson = snapshotWithDecisionActions([approve], { + status: { enum: ['open', 'closed'] }, + amount: { $ref: '#/$defs/money' }, + nickname: { type: ['string', 'null'] }, + }); + databaseMock.select.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson }])); + databaseMock.update.mockReturnValue(chainResolving([{ ...fakeWorkflow, publishedJson: draftJson }])); + + const response = await publish(allowAllApp()); + + expect(response.status).toBe(200); + expect(databaseMock.update).toHaveBeenCalledTimes(1); + }); + + it('still publishes a workflow without a draft, unvalidated', async () => { + databaseMock.select.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: null }])); + databaseMock.update.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: null }])); + + const response = await publish(allowAllApp()); + + expect(response.status).toBe(200); + expect(databaseMock.update).toHaveBeenCalledTimes(1); + }); + + // The falsy scalars used to short-circuit execute into published_version_missing while + // publish went on to validate them; only null means "no version". + it.each([ + { name: 'a broken decision request', draftJson: twoResumesSnapshot }, + { name: 'an empty string', draftJson: '' }, + { name: 'zero', draftJson: 0 }, + { name: 'false', draftJson: false }, + ])('answers with the same body execute gives for $name', async ({ draftJson }) => { + databaseMock.select.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson }])); + + const publishResponse = await publish(allowAllApp()); + const publishBody = await publishResponse.json(); + const executeResponse = await jsonRequest(allowAllApp(), '/api/workflows/w-1/execute', 'POST', { + sourceVersion: 'draft', + }); + + expect(executeResponse.status).toBe(400); + expect(await executeResponse.json()).toEqual(publishBody); + expect(databaseMock.insert).not.toHaveBeenCalled(); + expect(engineMock.submit).not.toHaveBeenCalled(); + }); +}); + +describe('createWorkflowsRoutes - draft save never validates the snapshot', () => { + it('stores a draft with a broken decision request', async () => { + databaseMock.update.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: twoResumesSnapshot }])); + + const response = await jsonRequest(allowAllApp(), '/api/workflows/w-1/draft', 'PATCH', { + draftJson: twoResumesSnapshot, + }); + + expect(response.status).toBe(200); + expect(databaseMock.update).toHaveBeenCalledTimes(1); + }); +}); + +// ---- own __proto__ keys in a stored draft ------------------------------------- +// A draft is stored as sent, so it can carry one. + +const poisonedDraft = JSON.parse( + '{"nodes":[{"id":"n1","data":{"type":"product/any","properties":' + + '{"__proto__":{"decisionRequest":{"version":99,"actions":[{"effect":"bogus"}],"schema":"x"}}}}}],"edges":[]}', +); + +describe('createWorkflowsRoutes - own __proto__ key in the draft', () => { + it('publish answers 400 invalid_snapshot pointing at the key and writes nothing', async () => { + databaseMock.select.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: poisonedDraft }])); + + const response = await publish(allowAllApp()); + const body = (await response.json()) as InvalidSnapshotBody; + + expect(response.status).toBe(400); + expect(body.code).toBe('invalid_snapshot'); + expect(body.details.map((detail) => detail.path.join('.'))).toEqual(['nodes.0.data.properties.__proto__']); + expect(databaseMock.update).not.toHaveBeenCalled(); + }); + + it('draft save still stores it', async () => { + databaseMock.update.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: poisonedDraft }])); + + const response = await jsonRequest(allowAllApp(), '/api/workflows/w-1/draft', 'PATCH', { + draftJson: poisonedDraft, + }); + + expect(response.status).toBe(200); + expect(databaseMock.update).toHaveBeenCalledTimes(1); + }); +}); + +// ---- deeply nested drafts ----------------------------------------------------- +// Depth is the client's to choose, and only the body limit caps it. + +const DEEP = 20_000; + +function deepDraft(leaf: string): unknown { + return JSON.parse( + '{"nodes":[{"id":"n1","data":{"type":"product/any","properties":{"deep":' + + '['.repeat(DEEP) + + leaf + + ']'.repeat(DEEP) + + '}}}],"edges":[]}', + ); +} + +describe('createWorkflowsRoutes - a draft nested deeper than a call stack', () => { + it('publishes a clean one', async () => { + databaseMock.select.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: deepDraft('') }])); + databaseMock.update.mockReturnValue(chainResolving([{ ...fakeWorkflow, draftJson: null }])); + + const response = await publish(allowAllApp()); + + expect(response.status).toBe(200); + expect(databaseMock.update).toHaveBeenCalledTimes(1); + }); + + it('refuses one hiding an own __proto__ key at the bottom, and points at it', async () => { + databaseMock.select.mockReturnValue( + chainResolving([{ ...fakeWorkflow, draftJson: deepDraft('{"__proto__":{}}') }]), + ); + + const response = await publish(allowAllApp()); + const body = (await response.json()) as InvalidSnapshotBody; + + expect(response.status).toBe(400); + expect(body.code).toBe('invalid_snapshot'); + expect(body.details[0]?.path.slice(0, 5)).toEqual(['nodes', 0, 'data', 'properties', 'deep']); + expect(body.details[0]?.path.at(-1)).toBe('__proto__'); + expect(databaseMock.update).not.toHaveBeenCalled(); + }); + + it('answers execute the same way', async () => { + databaseMock.select.mockReturnValue( + chainResolving([{ ...fakeWorkflow, draftJson: deepDraft('{"__proto__":{}}') }]), + ); + + const response = await jsonRequest(allowAllApp(), '/api/workflows/w-1/execute', 'POST', { + sourceVersion: 'draft', + }); + + expect(response.status).toBe(400); + expect(((await response.json()) as InvalidSnapshotBody).code).toBe('invalid_snapshot'); + expect(engineMock.submit).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/backend/src/routes/workflows.ts b/apps/backend/src/routes/workflows.ts index cb7906c85..283424c80 100644 --- a/apps/backend/src/routes/workflows.ts +++ b/apps/backend/src/routes/workflows.ts @@ -6,11 +6,11 @@ import type { AssertAuthorized, AuthVariables } from '../auth'; import { database } from '../db/client'; import { executions, workflows } from '../db/schema'; import { mapToExecutionModel } from '../domain/mapper/from-integration-data'; -import { workflowSnapshotSchema } from '../domain/mapper/snapshot-schema'; import { getWorkflowEngine } from '../engine'; import { logger as backendLogger } from '../logger'; import { guardExecution } from '../security/execution-guard'; import type { TenantVariables } from '../tenant'; +import { formatValidationDetails, parseSnapshot } from './snapshot-validation'; const logger = backendLogger.child({ component: 'workflows-route' }); @@ -28,14 +28,6 @@ const executeSchema = z.object({ triggerPayload: z.record(z.string(), z.unknown()).optional(), }); -function formatValidationDetails(error: z.ZodError) { - return error.issues.map((issue) => ({ - path: issue.path, - message: issue.message, - code: issue.code, - })); -} - export function createWorkflowsRoutes( assertAuthorized: AssertAuthorized, ): Hono<{ Variables: AuthVariables & TenantVariables }> { @@ -153,6 +145,12 @@ export function createWorkflowsRoutes( return c.json({ code: 'workflow_not_found', message: 'Workflow not found' }, 404); } + // A null draft is not validated; publishing it clears `publishedJson`, as before. + if (existing.draftJson !== null) { + const parsed = parseSnapshot(c, existing.draftJson, { workflowId, sourceVersion: 'draft' }); + if (parsed.response !== undefined) return parsed.response; + } + const [workflow] = await database .update(workflows) .set({ @@ -198,26 +196,12 @@ export function createWorkflowsRoutes( const snapshotJson = body.sourceVersion === 'published' ? workflow.publishedJson : workflow.draftJson; - if (!snapshotJson) { + if (snapshotJson === null) { return c.json({ code: 'published_version_missing', message: `No ${body.sourceVersion} version available` }, 400); } - const snapshotParsed = z.safeParse(workflowSnapshotSchema, snapshotJson); - if (!snapshotParsed.success) { - logger.warn('snapshot invalid', { - workflowId, - sourceVersion: body.sourceVersion, - error: { issues: formatValidationDetails(snapshotParsed.error) }, - }); - return c.json( - { - code: 'invalid_snapshot', - message: 'Workflow snapshot failed validation', - details: formatValidationDetails(snapshotParsed.error), - }, - 400, - ); - } + const snapshotParse = parseSnapshot(c, snapshotJson, { workflowId, sourceVersion: body.sourceVersion }); + if (snapshotParse.response !== undefined) return snapshotParse.response; // Propagate tenant identity from the HTTP boundary onto the execution row. // The worker reads it back via subquery for event tagging (see worker @@ -248,7 +232,7 @@ export function createWorkflowsRoutes( sourceVersion: body.sourceVersion, }); - const definition = mapToExecutionModel(workflowId, snapshotParsed.data); + const definition = mapToExecutionModel(workflowId, snapshotParse.snapshot); await getWorkflowEngine().submit({ workflowId, diff --git a/apps/backend/tenant-context-port.decision-log.md b/apps/backend/tenant-context-port.decision-log.md index ef937c2d8..6e8943976 100644 --- a/apps/backend/tenant-context-port.decision-log.md +++ b/apps/backend/tenant-context-port.decision-log.md @@ -2,7 +2,7 @@ ### Proposed by: Kacper Cierzniewski -### Proposed: 21.05.2026 — Landed: 03.06.2026 (`fa5999dd`) +### Date: 21.05.2026 (proposed; landed 03.06.2026 in `fa5999dd`) > This is the **decision** (why this shape, what was rejected, what it does and does not protect). The **how-to-wire-it** lives in [`multi-tenancy.md`](./multi-tenancy.md) — that document tracks the code and is the source of truth for current signatures and per-seam status. If a snippet here ever disagrees with the code, the code wins. diff --git a/packages/execution-core/src/graph-runner.test.ts b/packages/execution-core/src/graph-runner.test.ts index 047be5767..a4cfee2ac 100644 --- a/packages/execution-core/src/graph-runner.test.ts +++ b/packages/execution-core/src/graph-runner.test.ts @@ -1592,6 +1592,34 @@ describe('runGraph — node_started payload', () => { }); describe('runGraph — waiting results', () => { + // The runner reads no `decisionRequest`, deliberately: it learns no product's vocabulary. + // The field's public JSDoc and the backend README say so; this holds them to it. + it('runs a node carrying a decisionRequest like any other: only a waiting result parks', async () => { + const deciding: TestNode = { + id: 'B', + type: 'test/node', + config: {}, + decisionRequest: { + version: 1, + actions: [{ name: 'approve', label: 'Approve', effect: 'resume', port: 'approved' }], + schema: { type: 'object', properties: {} }, + }, + }; + const { port, callOrder } = makeRunner(); + const events = makeEvents(); + + const outcome = await runGraph( + makeInput([start('A'), deciding, trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + port, + events.port, + ); + + expect(outcome.status).toBe('completed'); + expect(callOrder).toEqual(['A', 'B', 'C']); + expect(events.events.map((event) => event.type)).not.toContain('node_waiting'); + expect(events.statuses.map((status) => status.status)).not.toContain('waiting'); + }); + it('fails the run when the adapter has no awaitResolution, even with errorPolicy continue on the gate', async () => { const runner = makeRunner({ A: { waits: true } }); const events = makeEvents(); diff --git a/packages/temporal/src/workflow/durable-pause.decision-log.md b/packages/temporal/src/workflow/durable-pause.decision-log.md index fe09d61e4..347e0994d 100644 --- a/packages/temporal/src/workflow/durable-pause.decision-log.md +++ b/packages/temporal/src/workflow/durable-pause.decision-log.md @@ -1,4 +1,8 @@ -# Durable pause (HITL seam) — decision log +### Title: Durable pause, the Temporal side of the human-in-the-loop seam + +### Proposed by: Piotr Błaszczyk + +### Date: 08.09.2026 Context: a node executor can return `{ waiting: true }`; the graph runner parks that wave slot on `ActivityRunnerPort.awaitResolution` and resumes with the completion the diff --git a/packages/types/src/workflow-execution/decision-request.ts b/packages/types/src/workflow-execution/decision-request.ts new file mode 100644 index 000000000..509fdd229 --- /dev/null +++ b/packages/types/src/workflow-execution/decision-request.ts @@ -0,0 +1,103 @@ +/** + * Effects a decision request may declare on its actions. `resume-with-edits` is deliberately absent: + * it is never declared; the backend derives it when a `resume` call carries edits. + */ +export const DECLARABLE_DECISION_EFFECTS = ['resume', 'reject', 'rerun-source'] as const; + +/** One of {@link DECLARABLE_DECISION_EFFECTS}. */ +export type DeclarableDecisionEffect = (typeof DECLARABLE_DECISION_EFFECTS)[number]; + +/** Every effect a decision can take, including the derived `resume-with-edits`. */ +export type DecisionEffect = DeclarableDecisionEffect | 'resume-with-edits'; + +type DecisionActionBase = { + /** + * What a submitted decision names. Unique within the request. Any string: the client's + * vocabulary, not an engine keyword. + */ + name: string; + /** Text shown to the decider. */ + label: string; +}; + +/** Accepts the proposal, edited or not: the run continues on `port`. Exactly one per request. */ +export type ResumeDecisionAction = DecisionActionBase & { + effect: 'resume'; + /** Output handle the run continues on. Defaults to `approved`. Never `errorRoute`. */ + port: string; +}; + +/** Rejects the proposal: the run continues on `port`. At most one per request. */ +export type RejectDecisionAction = DecisionActionBase & { + effect: 'reject'; + /** Output handle the run continues on. Defaults to `rejected`. Must differ from the resume port. */ + port: string; + /** Whether the decider must give a reason. Defaults to `false`. */ + reasonRequired: boolean; +}; + +/** Re-runs the proposal source with the decider's comment. At most one per request. */ +export type RerunSourceDecisionAction = DecisionActionBase & { + effect: 'rerun-source'; + /** Upper bound on re-runs of the proposal source. Integer of at least 1. Defaults to `3`. */ + maxIterations: number; +}; + +/** + * One action the decider can take, discriminated on `effect`. `port`, `reasonRequired` and + * `maxIterations` may be omitted in authored JSON; the backend parser materialises their + * defaults, so a parsed request always carries them. + */ +export type DecisionAction = ResumeDecisionAction | RejectDecisionAction | RerunSourceDecisionAction; + +/** Time limit on a node waiting for the decision. */ +export type DecisionDeadline = { + /** + * Counted from the moment the node parks. A number followed by `ms`, `s`, `m`, `h` or `d`, + * such as `'30s'` or `'3d'`. + */ + after: string; + /** What happens when `after` elapses. Typed open for future policies; only `'reject'` is accepted today. */ + policy: string; +}; + +/** + * A decision the request accepted: what is recorded on the node's completion and audited. + * `action` is the name of the chosen action; where it routes stays on the request, so the + * port is never written twice. `effect` is `resume-with-edits` when a resume carried edits. + */ +export type Decision = { + action: string; + effect: DecisionEffect; + edits: Record; + reason?: string; + comment?: string; +}; + +/** + * What a node asks a human to decide before the run continues. Authored under + * `data.properties.decisionRequest` in the editor snapshot and lifted to `BaseNode.decisionRequest`. + * Any node type may carry one. Unknown keys at every level are preserved. + */ +export type DecisionRequest = { + /** Shape version of the request. A future shape change bumps it. */ + version: 1; + /** Actions offered to the decider: exactly one `resume`, at most one `reject`, at most one `rerun-source`. */ + actions: DecisionAction[]; + /** + * JSON Schema of the decision form. `readOnly: true` marks a field the decider cannot + * edit; `x-pii: true` marks personal data. Opaque to the engine. + */ + schema: Record; + /** JsonForms UI schema for the decision form. Passed through; never read by the backend. */ + uiSchema?: Record; + /** + * The proposal source: the node whose output the decider judges. When set it must be a + * direct predecessor of the deciding node; when absent it is the node's single direct + * predecessor. Publishing refuses a request whose source does not resolve, so a published + * decision always names one node to judge. + */ + proposalSourceNodeId?: string; + /** Absent means the node waits forever. */ + deadline?: DecisionDeadline; +}; diff --git a/packages/types/src/workflow-execution/execution-model.ts b/packages/types/src/workflow-execution/execution-model.ts index ac5f38363..721f2d81e 100644 --- a/packages/types/src/workflow-execution/execution-model.ts +++ b/packages/types/src/workflow-execution/execution-model.ts @@ -1,3 +1,5 @@ +import type { DecisionRequest } from './decision-request'; + // Runner-level decision applied when a node throws. // `fail` aborts the whole execution (default); `continue` absorbs the error into // `nodeOutputs[id] = { error }` and propagates downstream; `errorRoute` does the @@ -32,6 +34,12 @@ export type BaseNode = { // without knowing any product's vocabulary. label?: string; errorPolicy?: NodeErrorPolicy; + /** + * What this node asks a human to decide before the run continues. The backend and the + * decision endpoint find it by this field, never by `type`. The runner does not read it: + * a run stops where a node's executor returns a waiting result. + */ + decisionRequest?: DecisionRequest; role?: NodeRole; };