From d54938a815315d161f439080b1a037ae3c6b5379 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:00:20 +0200 Subject: [PATCH 01/12] feat(worker): `bestEffort` for non-critical calls, rename `propagateActivityFailure` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #420, closes #423. `bestEffort(result, onFailure)` folds a call whose failure is not worth ending the workflow over — warn and carry on — while re-raising real cancellation through `rethrowCancellation`. The three near-identical best-effort blocks in the order-processing example were the motivation: each one had to remember, by hand, not to absorb a cancel. `propagateActivityFailure` becomes `propagateFailure`. Its own doc comment carried a "Not just activity calls" section explaining that it handles child-workflow calls and cancellation scopes too; the name now says so. The old export remains as a deprecated alias bound to the same function. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .../best-effort-and-propagate-rename.md | 16 +++ docs/explanation/nexus.md | 4 +- docs/explanation/the-result-model.md | 14 +- docs/explanation/workflow-determinism.md | 6 +- docs/how-to/continue-as-new.md | 8 +- docs/how-to/handle-cancellation.md | 12 +- docs/how-to/model-domain-errors.md | 2 +- docs/how-to/run-child-workflows.md | 6 +- docs/how-to/upgrade-to-v8.md | 12 +- .../how-to/use-signals-queries-and-updates.md | 6 +- docs/index.md | 6 +- docs/reference/errors.md | 2 +- docs/reference/worker-surface.md | 14 +- docs/tutorial/adding-signals-and-queries.md | 6 +- docs/tutorial/your-first-workflow.md | 8 +- .../src/application/workflows.ts | 10 +- .../src/__tests__/propagation.contract.ts | 2 +- .../src/__tests__/propagation.workflows.ts | 6 +- .../worker/src/__tests__/routing.workflows.ts | 4 +- .../worker/src/__tests__/test.workflows.ts | 22 ++-- .../src/__tests__/timeouts.workflows.ts | 6 +- packages/worker/src/activities-proxy.spec.ts | 2 +- packages/worker/src/activities-proxy.ts | 4 +- packages/worker/src/activity-failure.spec.ts | 122 ++++++++++++++---- packages/worker/src/activity-failure.ts | 70 +++++++++- packages/worker/src/cancellation.ts | 8 +- packages/worker/src/errors.ts | 4 +- packages/worker/src/saga.ts | 2 +- packages/worker/src/workflow.ts | 27 ++-- 29 files changed, 285 insertions(+), 126 deletions(-) create mode 100644 .changeset/best-effort-and-propagate-rename.md diff --git a/.changeset/best-effort-and-propagate-rename.md b/.changeset/best-effort-and-propagate-rename.md new file mode 100644 index 00000000..88b4e4f9 --- /dev/null +++ b/.changeset/best-effort-and-propagate-rename.md @@ -0,0 +1,16 @@ +--- +"@temporal-contract/worker": minor +--- + +`bestEffort(result, onFailure)` — the counterpart to `propagateFailure` for a +non-critical call (a notification, a metric, an audit write). It hands the +failure to `onFailure` and resolves `undefined` instead of ending the workflow, +but **re-raises real cancellation** (`ActivityCancelledError`, +`ChildWorkflowCancelledError`, `WorkflowCancelledError`) so a workflow can no +longer absorb its own cancel by accident. That rule used to live in every +hand-written best-effort fold; it is now structural. + +`propagateActivityFailure` is renamed to **`propagateFailure`** — it has always +also handled child-workflow calls and cancellation scopes, and the old name said +otherwise. The old name stays as a deprecated alias (the identical function +reference) and will be removed in the next major. diff --git a/docs/explanation/nexus.md b/docs/explanation/nexus.md index fbf68420..2762563d 100644 --- a/docs/explanation/nexus.md +++ b/docs/explanation/nexus.md @@ -69,7 +69,7 @@ covers and drop to the SDK at the Nexus boundary: ```typescript import * as nexus from "@temporalio/nexus"; -import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateFailure } from "@temporal-contract/worker/workflow"; const paymentService = nexus.service("PaymentService", { charge: nexus.operation<{ customerId: string; amount: number }, { transactionId: string }>(), @@ -81,7 +81,7 @@ export const processOrder = declareWorkflow({ activityOptions: { startToCloseTimeout: "1 minute", retry: { maximumAttempts: 3 } }, implementation: async (context, order) => { // Contract-typed for everything local... - const reserved = await propagateActivityFailure( + const reserved = await propagateFailure( context.activities.reserveInventory({ items: order.items }), ); diff --git a/docs/explanation/the-result-model.md b/docs/explanation/the-result-model.md index cd255ce1..4e391d31 100644 --- a/docs/explanation/the-result-model.md +++ b/docs/explanation/the-result-model.md @@ -95,24 +95,22 @@ if (charge.isErr()) { Most activity failures still have one sensible response: let Temporal's retry policy exhaust, then fail the workflow. Narrowing every such call site would add ceremony to code whose correct behaviour is "let it throw" — so use -`propagateActivityFailure` to re-raise the original failure and hand the +`propagateFailure` to re-raise the original failure and hand the outcome to Temporal, the same "let it throw" behaviour a bare `await` gave you before this call convention became uniform: ```typescript -import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { propagateFailure } from "@temporal-contract/worker/workflow"; -const charge = await propagateActivityFailure( - context.activities.chargeCard({ customerId, amount }), -); -const shipment = await propagateActivityFailure(context.activities.createShipment({ orderId })); +const charge = await propagateFailure(context.activities.chargeCard({ customerId, amount })); +const shipment = await propagateFailure(context.activities.createShipment({ orderId })); ``` **Do not use unthrown's `.getOrThrow()` for this.** It throws the `ActivityError`/`ActivityCancelledError` wrapper — a `TaggedError`, not a `TemporalFailure` — and Temporal treats a non-`TemporalFailure` thrown from workflow code as a workflow-_task_ failure, retrying it indefinitely rather -than failing the execution. `propagateActivityFailure` re-raises the +than failing the execution. `propagateFailure` re-raises the _preserved original_ Temporal failure instead, which is what actually fails the workflow. @@ -132,7 +130,7 @@ if (charged.isErr()) { ``` Declare errors on the activities whose failures should drive workflow -decisions; for the rest, `propagateActivityFailure` keeps the call site to a +decisions; for the rest, `propagateFailure` keeps the call site to a single line. ### Why child workflows never unwrap diff --git a/docs/explanation/workflow-determinism.md b/docs/explanation/workflow-determinism.md index 60b957a5..26c4fb20 100644 --- a/docs/explanation/workflow-determinism.md +++ b/docs/explanation/workflow-determinism.md @@ -152,12 +152,12 @@ Temporal's versioning API handles this: ```typescript import { patched } from "@temporalio/workflow"; -import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { propagateFailure } from "@temporal-contract/worker/workflow"; if (patched("add-fraud-check")) { - await propagateActivityFailure(context.activities.scoreRisk({ orderId })); // new path + await propagateFailure(context.activities.scoreRisk({ orderId })); // new path } -await propagateActivityFailure(context.activities.chargeCard({ ... })); // both paths +await propagateFailure(context.activities.chargeCard({ ... })); // both paths ``` `patched()` returns `true` for new executions and for old ones that already diff --git a/docs/how-to/continue-as-new.md b/docs/how-to/continue-as-new.md index e02cf3b4..aad1fd42 100644 --- a/docs/how-to/continue-as-new.md +++ b/docs/how-to/continue-as-new.md @@ -10,7 +10,7 @@ arguments and an empty history. ## The basic pattern ```typescript -import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateFailure } from "@temporal-contract/worker/workflow"; import { sleep } from "@temporalio/workflow"; export const pollSubscription = declareWorkflow({ @@ -19,7 +19,7 @@ export const pollSubscription = declareWorkflow({ activityOptions: { startToCloseTimeout: "1 minute", retry: { maximumAttempts: 3 } }, implementation: async (context, args) => { for (let i = 0; i < 100; i += 1) { - await propagateActivityFailure( + await propagateFailure( context.activities.chargeSubscription({ subscriptionId: args.subscriptionId }), ); await sleep("30 days"); @@ -53,12 +53,12 @@ implementation: async (context, args) => { let cursor = args.cursor; while (true) { - const batch = await propagateActivityFailure(context.activities.fetchBatch({ cursor })); + const batch = await propagateFailure(context.activities.fetchBatch({ cursor })); if (batch.items.length === 0) { return { processed }; } - await propagateActivityFailure(context.activities.processBatch({ items: batch.items })); + await propagateFailure(context.activities.processBatch({ items: batch.items })); processed += batch.items.length; cursor = batch.nextCursor; // advance, so the next fetch makes progress diff --git a/docs/how-to/handle-cancellation.md b/docs/how-to/handle-cancellation.md index bb135337..604b032f 100644 --- a/docs/how-to/handle-cancellation.md +++ b/docs/how-to/handle-cancellation.md @@ -127,22 +127,22 @@ Once a workflow is cancelled, further activity calls are cancelled too. Cleanup must run in a `nonCancellableScope`: ```typescript -import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { propagateFailure } from "@temporal-contract/worker/workflow"; implementation: async (context, order) => { let transactionId: string | undefined; const shipped = await context.cancellableScope(async () => { // Await and narrow the activity's own AsyncResult INSIDE the scope's - // callback — `propagateActivityFailure` lets a genuine (non-cancellation) + // callback — `propagateFailure` lets a genuine (non-cancellation) // charge failure ride the defect channel via the scope's own throw // handling, same as it would have without the scope. - const charge = await propagateActivityFailure( + const charge = await propagateFailure( context.activities.chargeCard({ customerId: order.customerId, amount: order.total }), ); transactionId = charge.transactionId; - return propagateActivityFailure(context.activities.createShipment({ orderId: order.orderId })); + return propagateFailure(context.activities.createShipment({ orderId: order.orderId })); }); if (shipped.isDefect()) { @@ -157,9 +157,7 @@ implementation: async (context, order) => { // narrowing across into this new arrow function. const chargedTransactionId = transactionId; const refunded = await context.nonCancellableScope(() => - propagateActivityFailure( - context.activities.refundPayment({ transactionId: chargedTransactionId }), - ), + propagateFailure(context.activities.refundPayment({ transactionId: chargedTransactionId })), ); if (refunded.isDefect()) { throw refunded.cause; // a refund that silently failed is worse than a loud failure diff --git a/docs/how-to/model-domain-errors.md b/docs/how-to/model-domain-errors.md index bdfaa37c..26360532 100644 --- a/docs/how-to/model-domain-errors.md +++ b/docs/how-to/model-domain-errors.md @@ -183,7 +183,7 @@ already seen through. Every activity call is already a `Result` — declaring errors doesn't add a result fold, it adds typed members to the one you already have. Declare errors on the activities whose failures the workflow actually branches on; for the -rest, `propagateActivityFailure` keeps the call site to a single line instead +rest, `propagateFailure` keeps the call site to a single line instead of a fold. See [The result model](/explanation/the-result-model). ::: diff --git a/docs/how-to/run-child-workflows.md b/docs/how-to/run-child-workflows.md index 7c32bf41..b5cb780e 100644 --- a/docs/how-to/run-child-workflows.md +++ b/docs/how-to/run-child-workflows.md @@ -12,7 +12,7 @@ same-contract and cross-contract calls look identical. `executeChildWorkflow` starts the child and waits for its result: ```typescript -import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateFailure } from "@temporal-contract/worker/workflow"; import { P } from "unthrown"; import { orderContract } from "./contract.js"; @@ -51,7 +51,7 @@ uniformity is deliberate. What differs is the usual _response_: a child workflow is a peer operation whose failure is usually a branch in your logic (narrow it, as above), whereas an activity failure is normally something Temporal's retry policy should already have handled by the time it reaches -the workflow (propagate it with `propagateActivityFailure`, unless the +the workflow (propagate it with `propagateFailure`, unless the workflow itself needs to branch on it too). See [The result model](/explanation/the-result-model). @@ -76,7 +76,7 @@ implementation: async (context, order) => { } // Do other work while the child runs. - const shipment = await propagateActivityFailure( + const shipment = await propagateFailure( context.activities.createShipment({ orderId: order.orderId }), ); diff --git a/docs/how-to/upgrade-to-v8.md b/docs/how-to/upgrade-to-v8.md index 443fc1d2..ce0ae394 100644 --- a/docs/how-to/upgrade-to-v8.md +++ b/docs/how-to/upgrade-to-v8.md @@ -726,17 +726,17 @@ if (result.isErr()) { // Or propagate it — let a failure escape and have Temporal decide the // workflow's fate, matching the pre-8.0 "just let it throw" behavior. -import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { propagateFailure } from "@temporal-contract/worker/workflow"; -await propagateActivityFailure(context.activities.sendEmail(input)); +await propagateFailure(context.activities.sendEmail(input)); ``` -**Do not reach for unthrown's `.getOrThrow()` instead of `propagateActivityFailure`.** +**Do not reach for unthrown's `.getOrThrow()` instead of `propagateFailure`.** `.getOrThrow()` throws the `ActivityError`/`ActivityCancelledError` wrapper itself — a `TaggedError`, not a `TemporalFailure`. Temporal treats a thrown non-`TemporalFailure` as a workflow-_task_ failure and retries it indefinitely, so the workflow never fails — it stalls until its execution -timeout instead. `propagateActivityFailure` re-raises the preserved original +timeout instead. `propagateFailure` re-raises the preserved original failure instead, which is exactly what would have escaped the workflow before this change. See [The result model](/explanation/the-result-model). @@ -744,7 +744,7 @@ A bare `await` that discards the result is easy to introduce by habit, especially copying a pre-8.0 call site that never needed narrowing. Grep for `await context.activities.` / `await activities.` (or your local alias) and confirm each hit either narrows the result or passes it through -`propagateActivityFailure` — an un-narrowed, un-propagated `AsyncResult` sitting +`propagateFailure` — an un-narrowed, un-propagated `AsyncResult` sitting in an expression statement is the tell. ### Cancellation can be swallowed by any activity call @@ -1129,7 +1129,7 @@ names the input. (`WorkflowCancelledError` / `Terminated` / `Timeout`, `UpdateFailedError`, `UpdateRejectedError`, `QueryFailedError`) - [ ] Every `await context.activities.x(...)` (declared-error or not) either - narrows the `AsyncResult` or is wrapped in `propagateActivityFailure` — + narrows the `AsyncResult` or is wrapped in `propagateFailure` — a bare, discarded `await` compiles identically before and after 8.0 but now silently swallows the failure - [ ] Cancellation isn't swallowed by **any** activity call (declared-error diff --git a/docs/how-to/use-signals-queries-and-updates.md b/docs/how-to/use-signals-queries-and-updates.md index aa327928..a49bddf6 100644 --- a/docs/how-to/use-signals-queries-and-updates.md +++ b/docs/how-to/use-signals-queries-and-updates.md @@ -89,7 +89,7 @@ Register handlers **inside** the implementation so they can close over workflow state: ```typescript -import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateFailure } from "@temporal-contract/worker/workflow"; import { condition } from "@temporalio/workflow"; export const importCatalog = declareWorkflow({ @@ -98,7 +98,7 @@ export const importCatalog = declareWorkflow({ activityOptions: { startToCloseTimeout: "5 minutes", retry: { maximumAttempts: 3 } }, implementation: async (context, args) => { let completed = 0; - let pending = await propagateActivityFailure( + let pending = await propagateFailure( context.activities.listSkus({ catalogId: args.catalogId }), ); let cancelReason: string | undefined; @@ -123,7 +123,7 @@ export const importCatalog = declareWorkflow({ while (pending.length > 0 && cancelReason === undefined) { const [next, ...rest] = pending; pending = rest; - await propagateActivityFailure(context.activities.importSku({ sku: next })); + await propagateFailure(context.activities.importSku({ sku: next })); completed += 1; } diff --git a/docs/index.md b/docs/index.md index 24c6fed9..9ad52ef9 100644 --- a/docs/index.md +++ b/docs/index.md @@ -96,7 +96,7 @@ export const activities = declareActivitiesHandler({ ``` ```typescript [3. Workflow] -import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateFailure } from "@temporal-contract/worker/workflow"; import { orderContract } from "./contract.js"; @@ -106,9 +106,9 @@ export const processOrder = declareWorkflow({ activityOptions: { startToCloseTimeout: "1 minute", retry: { maximumAttempts: 3 } }, implementation: async (context, order) => { // `order` is typed from the contract. So is the return value. Every - // activity call returns an AsyncResult; `propagateActivityFailure` lets + // activity call returns an AsyncResult; `propagateFailure` lets // Temporal's retry policy decide the outcome. - const { transactionId } = await propagateActivityFailure( + const { transactionId } = await propagateFailure( context.activities.chargeCard({ customerId: order.customerId, amount: order.amount, diff --git a/docs/reference/errors.md b/docs/reference/errors.md index 6e9cdbe7..0260f225 100644 --- a/docs/reference/errors.md +++ b/docs/reference/errors.md @@ -301,7 +301,7 @@ every non-cancellation failure lands here. | `cause` | the **unwrapped** actionable failure | | `originalFailure` | the failure exactly as caught, **before** the unwrap (typically Temporal's `ActivityFailure` wrapper) — `undefined` when there is no separate wrapper to retain | -`originalFailure` exists so `propagateActivityFailure` can re-raise the exact +`originalFailure` exists so `propagateFailure` can re-raise the exact failure Temporal originally produced without changing what `cause` means for existing consumers that narrow on it — see [Worker surface](/reference/worker-surface#propagateactivityfailure-result). diff --git a/docs/reference/worker-surface.md b/docs/reference/worker-surface.md index 445d2a31..05de6580 100644 --- a/docs/reference/worker-surface.md +++ b/docs/reference/worker-surface.md @@ -171,7 +171,7 @@ every activity, declared `errors` map or not: `ActivityErrorsFor` is `ActivityError | ActivityCancelledError`, plus the activity's declared `ContractErrorUnion` when it has one. Input is validated before the call, output after. See [The result model](/explanation/the-result-model) and -`propagateActivityFailure` below. +`propagateFailure` below. The map's type is `WorkflowInferWorkflowContextActivities` and a single entry's is `WorkflowInferActivity` — @@ -350,7 +350,7 @@ step did before saying no is knowable. They do **not** run on an `ActivityError`, a `ChildWorkflowError` or a defect: a step that failed unmodelled left state nobody can see, and un-deciding what you cannot see is a second bug. That failure propagates untouched, so -[`propagateActivityFailure`](#propagateactivityfailure-result) still re-raises +[`propagateFailure`](#propagateactivityfailure-result) still re-raises Temporal's original failure. Cancellation is the one case a caller may opt back in to, with @@ -410,10 +410,10 @@ Each `ValidationError` subclass carries a readonly `direction: "input" | "output"` field (the class names are unchanged; they remain `ApplicationFailure` subclasses discriminated by `failure.type`). -#### `propagateActivityFailure(result)` +#### `propagateFailure(result)` ```typescript -function propagateActivityFailure(result: AsyncResult): Promise; +function propagateFailure(result: AsyncResult): Promise; ``` Await an activity call and return its value, re-raising the original Temporal @@ -421,9 +421,9 @@ failure so **Temporal** decides the workflow's outcome — the explicit equivalent of the pre-8.0 "just let it throw" call site: ```typescript -import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { propagateFailure } from "@temporal-contract/worker/workflow"; -const { transactionId } = await propagateActivityFailure( +const { transactionId } = await propagateFailure( context.activities.chargeCard({ customerId, amount }), ); ``` @@ -432,7 +432,7 @@ const { transactionId } = await propagateActivityFailure( `ActivityError`/`ActivityCancelledError` wrapper — a `TaggedError`, not a `TemporalFailure` — which Temporal treats as a workflow-_task_ failure and retries indefinitely, stalling the workflow until its execution timeout -instead of failing it. `propagateActivityFailure` re-raises the preserved +instead of failing it. `propagateFailure` re-raises the preserved original failure instead — see [The result model](/explanation/the-result-model). diff --git a/docs/tutorial/adding-signals-and-queries.md b/docs/tutorial/adding-signals-and-queries.md index e07bb6f3..759c0266 100644 --- a/docs/tutorial/adding-signals-and-queries.md +++ b/docs/tutorial/adding-signals-and-queries.md @@ -111,7 +111,7 @@ Edit `src/workflows.ts`. The workflow now holds mutable state, registers three handlers, and waits for approval before charging: ```typescript -import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateFailure } from "@temporal-contract/worker/workflow"; import { condition } from "@temporalio/workflow"; import { orderContract } from "./contract.js"; @@ -151,14 +151,14 @@ export const processOrder = declareWorkflow({ state = "charging"; - const { transactionId } = await propagateActivityFailure( + const { transactionId } = await propagateFailure( context.activities.chargeCard({ customerId: order.customerId, amount, }), ); - await propagateActivityFailure( + await propagateFailure( context.activities.sendReceipt({ customerId: order.customerId, transactionId }), ); diff --git a/docs/tutorial/your-first-workflow.md b/docs/tutorial/your-first-workflow.md index d5ec15d5..d0bd3454 100644 --- a/docs/tutorial/your-first-workflow.md +++ b/docs/tutorial/your-first-workflow.md @@ -237,7 +237,7 @@ The workflow orchestrates. It must be deterministic — no `Date.now()`, no Create `src/workflows.ts`: ```typescript -import { declareWorkflow, propagateActivityFailure } from "@temporal-contract/worker/workflow"; +import { declareWorkflow, propagateFailure } from "@temporal-contract/worker/workflow"; import { orderContract } from "./contract.js"; @@ -252,14 +252,14 @@ export const processOrder = declareWorkflow({ retry: { maximumAttempts: 3 }, }, implementation: async (context, order) => { - const { transactionId } = await propagateActivityFailure( + const { transactionId } = await propagateFailure( context.activities.chargeCard({ customerId: order.customerId, amount: order.amount, }), ); - await propagateActivityFailure( + await propagateFailure( context.activities.sendReceipt({ customerId: order.customerId, transactionId }), ); @@ -274,7 +274,7 @@ object and TypeScript will tell you it no longer satisfies the contract. Notice that `context.activities.chargeCard(...)` returns an **`AsyncResult`**, not a plain value — every activity call does, whether or not the contract -declares any `errors`. `propagateActivityFailure` unwraps the success value +declares any `errors`. `propagateFailure` unwraps the success value and re-raises the original failure on the way out, so Temporal's retry policy still handles it — the same "let it throw" behavior as before, made explicit at the call site. See [The result model](/explanation/the-result-model). diff --git a/examples/order-processing-worker/src/application/workflows.ts b/examples/order-processing-worker/src/application/workflows.ts index a206e579..52e4028b 100644 --- a/examples/order-processing-worker/src/application/workflows.ts +++ b/examples/order-processing-worker/src/application/workflows.ts @@ -6,7 +6,7 @@ import { ACTIVITY_CANCELLED_ERROR_TAG, ACTIVITY_ERROR_TAG, declareWorkflow, - propagateActivityFailure, + propagateFailure, rethrowCancellation, } from "@temporal-contract/worker/workflow"; import { condition, log } from "@temporalio/workflow"; @@ -289,13 +289,13 @@ export const processOrder = declareWorkflow({ // customer would be charged for an order that both failed and was // never refunded. That is exactly the case where Temporal should fail // the workflow loudly (visible, alertable) instead of completing it - // with a routine "failed" order status. `propagateActivityFailure` + // with a routine "failed" order status. `propagateFailure` // restores the exact pre-uniform-`AsyncResult` behavior: before every // activity call returned a `Result`, an unhandled `refundPayment` // failure threw and failed this workflow outright — this is that same // outcome, made explicit instead of accidental. log.info("Rolling back: refunding payment"); - await propagateActivityFailure(activities.refundPayment(payment.transactionId)); + await propagateFailure(activities.refundPayment(payment.transactionId)); log.info(`Payment refunded: ${payment.transactionId}`); // Best-effort notification — see the PaymentDeclined branch above for @@ -350,7 +350,7 @@ export const processOrder = declareWorkflow({ // No rollback path exists for a failed shipment creation (unlike // inventory reservation above) — that is a genuine "let Temporal fail // the workflow" case, not a business outcome this example models. - const shippingResult = await propagateActivityFailure( + const shippingResult = await propagateFailure( activities.createShipment({ orderId: order.orderId, customerId: order.customerId, @@ -443,7 +443,7 @@ export const cleanupExpiredOrders = declareWorkflow({ // fail loudly (visible in the Temporal UI, the schedule runs again next // time) rather than being silently swallowed into a fake "0 purged" // success. - const { purgedCount } = await propagateActivityFailure( + const { purgedCount } = await propagateFailure( context.activities.purgeExpiredOrders({ olderThanDays }), ); diff --git a/packages/worker/src/__tests__/propagation.contract.ts b/packages/worker/src/__tests__/propagation.contract.ts index 9098782c..e3ad0d5a 100644 --- a/packages/worker/src/__tests__/propagation.contract.ts +++ b/packages/worker/src/__tests__/propagation.contract.ts @@ -7,7 +7,7 @@ import { z } from "zod"; * (now-deleted) `makeThrowingActivity` path, and Temporal's original * `ActivityFailure` propagated out of the workflow via a bare `await`. This * is the activity whose observable behavior — now reached through - * `propagateActivityFailure`, or handled by narrowing `isErr()` — must stay + * `propagateFailure`, or handled by narrowing `isErr()` — must stay * IDENTICAL to that pre-change throwing behavior. * * `maximumAttempts: 2` bounds the run: enough to prove Temporal retried, diff --git a/packages/worker/src/__tests__/propagation.workflows.ts b/packages/worker/src/__tests__/propagation.workflows.ts index 04303494..12f1d5a6 100644 --- a/packages/worker/src/__tests__/propagation.workflows.ts +++ b/packages/worker/src/__tests__/propagation.workflows.ts @@ -1,11 +1,11 @@ import { ActivityFailure } from "@temporalio/workflow"; import { ActivityError } from "../errors.js"; -import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; +import { declareWorkflow, propagateFailure } from "../workflow.js"; import { propagationContract } from "./propagation.contract.js"; /** - * Lets the failure escape via `propagateActivityFailure`, the post-change + * Lets the failure escape via `propagateFailure`, the post-change * equivalent of the bare `await` this fixture used before. The * characterization spec asserts the workflow still FAILS and that Temporal * still retried the activity to its configured maximum. @@ -14,7 +14,7 @@ export const propagatesFailure = declareWorkflow({ workflowName: "propagatesFailure", contract: propagationContract, implementation: async (context) => { - await propagateActivityFailure(context.activities.alwaysFailsNoErrors({})); + await propagateFailure(context.activities.alwaysFailsNoErrors({})); return { reached: true }; }, }); diff --git a/packages/worker/src/__tests__/routing.workflows.ts b/packages/worker/src/__tests__/routing.workflows.ts index db8689fa..b30b8f33 100644 --- a/packages/worker/src/__tests__/routing.workflows.ts +++ b/packages/worker/src/__tests__/routing.workflows.ts @@ -1,4 +1,4 @@ -import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; +import { declareWorkflow, propagateFailure } from "../workflow.js"; import { ROUTED_ACTIVITY_QUEUE, routingContract } from "./routing.contract.js"; /** @@ -22,7 +22,7 @@ export const routedFlow = declareWorkflow({ // worker always resolves); a failure here would be an unmodeled routing // bug, so let Temporal decide the workflow's fate rather than folding it // into a returned status. - const { handledBy } = await propagateActivityFailure(context.activities.reportQueue({})); + const { handledBy } = await propagateFailure(context.activities.reportQueue({})); return { handledBy }; }, }); diff --git a/packages/worker/src/__tests__/test.workflows.ts b/packages/worker/src/__tests__/test.workflows.ts index bbc05578..f583a998 100644 --- a/packages/worker/src/__tests__/test.workflows.ts +++ b/packages/worker/src/__tests__/test.workflows.ts @@ -1,13 +1,13 @@ import { sleep } from "@temporalio/workflow"; -import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; +import { declareWorkflow, propagateFailure } from "../workflow.js"; import { testContract } from "./test.contract.js"; export const simpleWorkflow = declareWorkflow({ workflowName: "simpleWorkflow", contract: testContract, implementation: async ({ activities }, args) => { - await propagateActivityFailure(activities.logMessage({ message: `Processing: ${args.value}` })); + await propagateFailure(activities.logMessage({ message: `Processing: ${args.value}` })); return { result: `Processed: ${args.value}`, }; @@ -35,10 +35,10 @@ export const workflowWithActivities = declareWorkflow({ // value, not activity failures. A *technical* failure here (neither // test exercises one) should still fail the workflow rather than being // folded into the "failed" business status, so unwrap with - // propagateActivityFailure and only branch on the business fields. + // propagateFailure and only branch on the business fields. // Validate order - const validationResult = await propagateActivityFailure( + const validationResult = await propagateFailure( activities.validateOrder({ orderId: args.orderId }), ); @@ -51,7 +51,7 @@ export const workflowWithActivities = declareWorkflow({ } // Process payment - const paymentResult = await propagateActivityFailure( + const paymentResult = await propagateFailure( activities.processPayment({ amount: args.amount }), ); @@ -64,7 +64,7 @@ export const workflowWithActivities = declareWorkflow({ } // Log success - await propagateActivityFailure( + await propagateFailure( activities.logMessage({ message: `Order ${args.orderId} completed with transaction ${paymentResult.transactionId}`, }), @@ -152,9 +152,7 @@ export const childWorkflow = declareWorkflow({ workflowName: "childWorkflow", contract: testContract, implementation: async ({ activities }, args) => { - await propagateActivityFailure( - activities.logMessage({ message: `Child workflow ${args.id} running` }), - ); + await propagateFailure(activities.logMessage({ message: `Child workflow ${args.id} running` })); return { message: `Child ${args.id} completed`, }; @@ -172,10 +170,8 @@ export const workflowWithFailableActivity = declareWorkflow({ // The (skipped) "Error Handling" spec in worker.spec.ts expects the // workflow itself to FAIL when the activity fails — not to fold the // failure into a returned status — so let it escape via - // propagateActivityFailure rather than narrowing. - return await propagateActivityFailure( - activities.failableActivity({ shouldFail: args.shouldFail }), - ); + // propagateFailure rather than narrowing. + return await propagateFailure(activities.failableActivity({ shouldFail: args.shouldFail })); }, activityOptions: { startToCloseTimeout: "1 minute", diff --git a/packages/worker/src/__tests__/timeouts.workflows.ts b/packages/worker/src/__tests__/timeouts.workflows.ts index cbc31aba..df8ea9d9 100644 --- a/packages/worker/src/__tests__/timeouts.workflows.ts +++ b/packages/worker/src/__tests__/timeouts.workflows.ts @@ -1,4 +1,4 @@ -import { declareWorkflow, propagateActivityFailure } from "../workflow.js"; +import { declareWorkflow, propagateFailure } from "../workflow.js"; import { timeoutsContract } from "./timeouts.contract.js"; export const reportsLayered = declareWorkflow({ @@ -17,7 +17,7 @@ export const reportsLayered = declareWorkflow({ // call is uniformly `AsyncResult`-shaped now regardless of a declared // `errors` map. The spec only exercises the success path (each merge // layer contributing its value), so let a technical failure escape via - // propagateActivityFailure and have Temporal decide the workflow's fate. - return await propagateActivityFailure(context.activities.reportsTimeouts({})); + // propagateFailure and have Temporal decide the workflow's fate. + return await propagateFailure(context.activities.reportsTimeouts({})); }, }); diff --git a/packages/worker/src/activities-proxy.spec.ts b/packages/worker/src/activities-proxy.spec.ts index 29bc19e8..4f69b36a 100644 --- a/packages/worker/src/activities-proxy.spec.ts +++ b/packages/worker/src/activities-proxy.spec.ts @@ -208,7 +208,7 @@ describe("createValidatedActivities — activities with declared errors", () => // cause stays the UNWRAPPED failure (documented, unchanged behavior). expect(error.cause).toBe(inner); // originalFailure is the wrapper Temporal actually threw, retained - // specifically so propagateActivityFailure can re-raise it faithfully. + // specifically so propagateFailure can re-raise it faithfully. expect(error.originalFailure).toBe(wrapper); } }); diff --git a/packages/worker/src/activities-proxy.ts b/packages/worker/src/activities-proxy.ts index ddb963b7..a59a08b1 100644 --- a/packages/worker/src/activities-proxy.ts +++ b/packages/worker/src/activities-proxy.ts @@ -55,7 +55,7 @@ export type ActivityErrorsFor = TActivity * Every activity call returns an `AsyncResult` — the call convention no * longer depends on whether the contract declared errors, only the error * channel does. To let a failure escape and have Temporal decide the - * workflow's outcome, use `propagateActivityFailure` rather than + * workflow's outcome, use `propagateFailure` rather than * unthrown's `.getOrThrow()`; see that function's documentation for why. */ export type WorkflowInferActivity = ( @@ -252,7 +252,7 @@ async function classifyActivityError( `Activity "${activityName}" failed: ${innerMessage}`, inner, // Retain the value exactly as caught (pre-unwrap) as `originalFailure`, - // alongside the unwrapped `cause` above. `propagateActivityFailure` + // alongside the unwrapped `cause` above. `propagateFailure` // re-raises `originalFailure` so Temporal classifies the workflow // outcome exactly as it would if this activity call still threw // directly — see the field's doc comment on `ActivityError`. diff --git a/packages/worker/src/activity-failure.spec.ts b/packages/worker/src/activity-failure.spec.ts index ee479e53..8b59394f 100644 --- a/packages/worker/src/activity-failure.spec.ts +++ b/packages/worker/src/activity-failure.spec.ts @@ -3,7 +3,7 @@ import { ApplicationFailure, ActivityFailure, RetryState } from "@temporalio/com import { ErrAsync, OkAsync } from "unthrown"; import { describe, expect, it } from "vitest"; -import { propagateActivityFailure } from "./activity-failure.js"; +import { bestEffort, propagateActivityFailure, propagateFailure } from "./activity-failure.js"; import { ActivityCancelledError, ActivityError, @@ -14,9 +14,9 @@ import { WorkflowCancelledError, } from "./errors.js"; -describe("propagateActivityFailure", () => { +describe("propagateFailure", () => { it("returns the value on Ok", async () => { - await expect(propagateActivityFailure(OkAsync({ ok: true }))).resolves.toEqual({ ok: true }); + await expect(propagateFailure(OkAsync({ ok: true }))).resolves.toEqual({ ok: true }); }); it("rethrows the ORIGINAL ActivityFailure wrapper, not the unwrapped cause", async () => { @@ -43,14 +43,14 @@ describe("propagateActivityFailure", () => { wrapper, ); - await expect(propagateActivityFailure(ErrAsync(activityError))).rejects.toBe(wrapper); + await expect(propagateFailure(ErrAsync(activityError))).rejects.toBe(wrapper); }); it("falls back to cause when no originalFailure was preserved", async () => { const cause = ApplicationFailure.create({ message: "boom", type: "Boom" }); const activityError = new ActivityError("charge", 'Activity "charge" failed: boom', cause); - await expect(propagateActivityFailure(ErrAsync(activityError))).rejects.toBe(cause); + await expect(propagateFailure(ErrAsync(activityError))).rejects.toBe(cause); }); it("rethrows the wrapper itself when neither cause nor originalFailure was preserved", async () => { @@ -58,7 +58,7 @@ describe("propagateActivityFailure", () => { // ActivityError itself is the most informative thing available. const activityError = new ActivityError("charge", 'Activity "charge" failed: opaque'); - await expect(propagateActivityFailure(ErrAsync(activityError))).rejects.toBe(activityError); + await expect(propagateFailure(ErrAsync(activityError))).rejects.toBe(activityError); }); it("rethrows the preserved cause for a cancelled activity", async () => { @@ -68,18 +68,18 @@ describe("propagateActivityFailure", () => { const cancelledFailure = ApplicationFailure.create({ message: "cancelled", type: "Cancelled" }); const cancelled = new ActivityCancelledError("charge", cancelledFailure); - await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); + await expect(propagateFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); }); it("rethrows a cancelled activity's wrapper when no cause was preserved", async () => { const cancelled = new ActivityCancelledError("charge"); - await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelled); + await expect(propagateFailure(ErrAsync(cancelled))).rejects.toBe(cancelled); }); it("rethrows a non-ActivityError error value unchanged", async () => { const other = new Error("something else"); - await expect(propagateActivityFailure(ErrAsync(other))).rejects.toBe(other); + await expect(propagateFailure(ErrAsync(other))).rejects.toBe(other); }); it("rethrows the ApplicationFailure cause for a declared ContractError, not the TaggedError wrapper", async () => { @@ -106,7 +106,7 @@ describe("propagateActivityFailure", () => { }); expect(contractError).not.toBeInstanceOf(ApplicationFailure); - await expect(propagateActivityFailure(ErrAsync(contractError))).rejects.toBe(wireFailure); + await expect(propagateFailure(ErrAsync(contractError))).rejects.toBe(wireFailure); }); it("rethrows a ContractError itself when no cause was set", async () => { @@ -116,7 +116,7 @@ describe("propagateActivityFailure", () => { message: "Card declined", }); - await expect(propagateActivityFailure(ErrAsync(contractError))).rejects.toBe(contractError); + await expect(propagateFailure(ErrAsync(contractError))).rejects.toBe(contractError); }); it("rethrows the preserved cause for a failed child workflow", async () => { @@ -124,7 +124,7 @@ describe("propagateActivityFailure", () => { const cause = ApplicationFailure.create({ message: "child failed", type: "Boom" }); const childError = new ChildWorkflowError("processPayment", "Child workflow failed", cause); - await expect(propagateActivityFailure(ErrAsync(childError))).rejects.toBe(cause); + await expect(propagateFailure(ErrAsync(childError))).rejects.toBe(cause); }); it("converts a causeless child workflow error to a terminal ContractMisuseError, not a bare TaggedError rethrow", async () => { @@ -136,10 +136,8 @@ describe("propagateActivityFailure", () => { // ApplicationFailure instead (see activity-failure.ts's doc comment). const childError = new ChildWorkflowError("processPayment", "Child workflow failed"); - await expect(propagateActivityFailure(ErrAsync(childError))).rejects.toThrow( - ContractMisuseError, - ); - await expect(propagateActivityFailure(ErrAsync(childError))).rejects.toMatchObject({ + await expect(propagateFailure(ErrAsync(childError))).rejects.toThrow(ContractMisuseError); + await expect(propagateFailure(ErrAsync(childError))).rejects.toMatchObject({ message: childError.message, nonRetryable: true, }); @@ -149,7 +147,7 @@ describe("propagateActivityFailure", () => { const cancelledFailure = ApplicationFailure.create({ message: "cancelled", type: "Cancelled" }); const cancelled = new ChildWorkflowCancelledError("processPayment", cancelledFailure); - await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); + await expect(propagateFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); }); it("rethrows the preserved cause for a cancelled cancellation scope", async () => { @@ -158,13 +156,13 @@ describe("propagateActivityFailure", () => { const cancelledFailure = ApplicationFailure.create({ message: "cancelled", type: "Cancelled" }); const cancelled = new WorkflowCancelledError(cancelledFailure); - await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); + await expect(propagateFailure(ErrAsync(cancelled))).rejects.toBe(cancelledFailure); }); it("rethrows a cancelled scope's wrapper when no cause was preserved", async () => { const cancelled = new WorkflowCancelledError(); - await expect(propagateActivityFailure(ErrAsync(cancelled))).rejects.toBe(cancelled); + await expect(propagateFailure(ErrAsync(cancelled))).rejects.toBe(cancelled); }); it("converts a not-found child workflow to a terminal ContractMisuseError, not a bare TaggedError rethrow", async () => { @@ -175,8 +173,8 @@ describe("propagateActivityFailure", () => { // converted to a terminal ApplicationFailure instead. const notFound = new ChildWorkflowNotFoundError("processPayment", ["processOrder"]); - await expect(propagateActivityFailure(ErrAsync(notFound))).rejects.toThrow(ContractMisuseError); - await expect(propagateActivityFailure(ErrAsync(notFound))).rejects.toMatchObject({ + await expect(propagateFailure(ErrAsync(notFound))).rejects.toThrow(ContractMisuseError); + await expect(propagateFailure(ErrAsync(notFound))).rejects.toMatchObject({ message: notFound.message, nonRetryable: true, }); @@ -199,6 +197,86 @@ describe("propagateActivityFailure", () => { throw activityError; }); - await expect(propagateActivityFailure(defect)).rejects.toBe(cause); + await expect(propagateFailure(defect)).rejects.toBe(cause); + }); +}); + +describe("propagateActivityFailure (deprecated alias)", () => { + it("is the same function as propagateFailure", () => { + // Not a behavioural copy — the identical reference, so the alias cannot + // drift from the helper it stands in for. + expect(propagateActivityFailure).toBe(propagateFailure); + }); +}); + +describe("bestEffort", () => { + it("returns the value and never calls onFailure on Ok", async () => { + const seen: unknown[] = []; + + await expect(bestEffort(OkAsync({ sent: true }), (f) => seen.push(f))).resolves.toEqual({ + sent: true, + }); + expect(seen).toEqual([]); + }); + + it("hands a modeled activity failure to onFailure and resolves undefined", async () => { + const cause = ApplicationFailure.create({ message: "smtp down", type: "NOTIFY_FAILED" }); + const failure = new ActivityError("sendNotification", "notify failed", cause); + const seen: unknown[] = []; + + await expect(bestEffort(ErrAsync(failure), (f) => seen.push(f))).resolves.toBeUndefined(); + expect(seen).toEqual([failure]); + }); + + it("hands a defect's cause to onFailure rather than rethrowing it", async () => { + // A best-effort call has already been declared non-critical: a bug in the + // notification path must not block an outcome that is already decided. + const cause = new TypeError("cannot read properties of undefined"); + const seen: unknown[] = []; + + await expect( + bestEffort( + OkAsync(undefined).map(() => { + throw cause; + }), + (f) => seen.push(f), + ), + ).resolves.toBeUndefined(); + expect(seen).toEqual([cause]); + }); + + it("re-raises a cancelled ACTIVITY call instead of absorbing it", async () => { + // The reason this helper exists. Absorbing cancellation would let the + // workflow run on to Completed after someone asked it to stop. + const cancelledFailure = ApplicationFailure.create({ message: "cancelled" }); + const cancelled = new ActivityCancelledError("sendNotification", cancelledFailure); + const seen: unknown[] = []; + + await expect(bestEffort(ErrAsync(cancelled), (f) => seen.push(f))).rejects.toBe( + cancelledFailure, + ); + expect(seen).toEqual([]); + }); + + it("re-raises a cancelled CHILD WORKFLOW call", async () => { + const cancelledFailure = ApplicationFailure.create({ message: "cancelled" }); + const cancelled = new ChildWorkflowCancelledError("childOrder", cancelledFailure); + const seen: unknown[] = []; + + await expect(bestEffort(ErrAsync(cancelled), (f) => seen.push(f))).rejects.toBe( + cancelledFailure, + ); + expect(seen).toEqual([]); + }); + + it("re-raises a cancelled SCOPE", async () => { + const cancelledFailure = ApplicationFailure.create({ message: "cancelled" }); + const cancelled = new WorkflowCancelledError(cancelledFailure); + const seen: unknown[] = []; + + await expect(bestEffort(ErrAsync(cancelled), (f) => seen.push(f))).rejects.toBe( + cancelledFailure, + ); + expect(seen).toEqual([]); }); }); diff --git a/packages/worker/src/activity-failure.ts b/packages/worker/src/activity-failure.ts index 753e1588..9362f02e 100644 --- a/packages/worker/src/activity-failure.ts +++ b/packages/worker/src/activity-failure.ts @@ -8,6 +8,7 @@ import { ChildWorkflowError, ChildWorkflowNotFoundError, ContractMisuseError, + rethrowCancellation, WorkflowCancelledError, } from "./errors.js"; @@ -80,8 +81,8 @@ import { * A failure with nothing preserved at all rethrows the wrapper, so the error * identity is never lost. * - * **Not just activity calls.** The same non-`TemporalFailure`-stall hazard - * applies to `context.executeChildWorkflow` / `context.startChildWorkflow` + * **Not just activity calls** — hence the name. The same + * non-`TemporalFailure`-stall hazard applies to `context.executeChildWorkflow` / `context.startChildWorkflow` * (`ChildWorkflowError`, `ChildWorkflowCancelledError`) and to * `context.cancellableScope` / `context.nonCancellableScope` * (`WorkflowCancelledError`, whose `cause` holds the original @@ -111,7 +112,7 @@ import { * `ContractMisuseError` (a non-retryable `ApplicationFailure`) instead, so * it still fails the workflow terminally rather than stalling it. */ -export async function propagateActivityFailure(result: AsyncResult): Promise { +export async function propagateFailure(result: AsyncResult): Promise { const settled = await result; if (settled.isOk()) { return settled.value; @@ -160,3 +161,66 @@ export async function propagateActivityFailure(result: AsyncResult): // oxlint-disable-next-line unthrown/no-throw -- deliberate re-raise: an unmodeled error/defect value is rethrown unchanged throw error; } + +/** + * @deprecated Renamed to {@link propagateFailure}: this helper has always + * handled child-workflow calls and cancellation scopes too, not just activity + * calls, and the old name said otherwise. Behaviourally identical; it will be + * removed in the next major. + */ +export const propagateActivityFailure = propagateFailure; + +/** + * Await a call whose failure is **not** worth ending the workflow over — a + * notification, a metric, an audit write — and hand that failure to + * `onFailure` instead. Returns the value on success and `undefined` on + * failure, so a caller that wants the value can still narrow it. + * + * The counterpart to {@link propagateFailure}: that one says "let Temporal + * decide", this one says "log it and carry on". + * + * **Cancellation is the exception, and that is the whole point of having this + * as a helper.** A cancelled call arrives on the modeled `Err` channel like + * any other failure, so a hand-written best-effort fold absorbs it — and a + * workflow that absorbs its own cancellation runs to `Completed` after + * someone asked it to stop. Every cancellation shape + * ({@link ActivityCancelledError}, {@link ChildWorkflowCancelledError}, + * {@link WorkflowCancelledError}) is re-raised through + * {@link rethrowCancellation} before `onFailure` is ever reached, so the + * rule is structural instead of remembered at each call site. + * + * A `Defect` (an unmodeled failure — a bug) is passed to `onFailure` like any + * other: the caller has already declared this call non-critical, and a + * notification bug must not block an outcome that is already authoritative. + * Reach for {@link propagateFailure} when that is not true. + * + * @example + * ```ts + * await bestEffort( + * context.activities.sendNotification({ customerId, subject, message }), + * (failure) => log.warn(`notification failed: ${String(failure)}`), + * ); + * ``` + */ +export async function bestEffort( + result: AsyncResult, + onFailure: (failure: unknown) => void, +): Promise { + const settled = await result; + if (settled.isOk()) { + return settled.value; + } + + const error: unknown = settled.isErr() ? settled.error : settled.cause; + + if ( + error instanceof ActivityCancelledError || + error instanceof ChildWorkflowCancelledError || + error instanceof WorkflowCancelledError + ) { + rethrowCancellation(error); + } + + onFailure(error); + return undefined; +} diff --git a/packages/worker/src/cancellation.ts b/packages/worker/src/cancellation.ts index 28d2fd21..742803ad 100644 --- a/packages/worker/src/cancellation.ts +++ b/packages/worker/src/cancellation.ts @@ -31,14 +31,14 @@ import { WorkflowCancelledError } from "./errors.js"; * @example * ```ts * import { P } from "unthrown"; - * import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; + * import { propagateFailure } from "@temporal-contract/worker/workflow"; * * // `fn`'s return value becomes the scope's `T` verbatim — an un-awaited * // `context.activities.processStep(...)` would make `T` the AsyncResult * // itself (it has no `isOk`/`isErr`/`.value`), not the activity's output. - * // `propagateActivityFailure` awaits it and hands the scope a plain value. + * // `propagateFailure` awaits it and hands the scope a plain value. * const result = await context.cancellableScope(async () => { - * return await propagateActivityFailure(context.activities.processStep(...)); + * return await propagateFailure(context.activities.processStep(...)); * }); * * result.match({ @@ -49,7 +49,7 @@ import { WorkflowCancelledError } from "./errors.js"; * }), * defect: (cause) => { * // a non-cancellation failure thrown inside the scope (a bug) — or, - * // via propagateActivityFailure, a non-cancellation activity failure + * // via propagateFailure, a non-cancellation activity failure * }, * }); * ``` diff --git a/packages/worker/src/errors.ts b/packages/worker/src/errors.ts index 3a5a2745..aae7aede 100644 --- a/packages/worker/src/errors.ts +++ b/packages/worker/src/errors.ts @@ -328,7 +328,7 @@ export class ContractMisuseError extends ValidationError { * was caught, *before* `classifyActivityError` unwrapped it into `cause` * (typically Temporal's `ActivityFailure` wrapper). `cause`'s unwrapping is * documented, caller-facing behavior and stays as-is — `originalFailure` - * exists purely so {@link propagateActivityFailure} can re-raise the exact + * exists purely so {@link propagateFailure} can re-raise the exact * failure Temporal originally produced, without changing what `cause` means. * Unset when there is no separate wrapper to retain (e.g. the input/output * validation branches, where `cause` is already the terminal failure). @@ -366,7 +366,7 @@ export class ActivityError extends TaggedError(ACTIVITY_ERROR_TAG, { * Unlike {@link ActivityError}, `cause` here is already the value exactly as * caught (`classifyActivityError` checks cancellation *before* unwrapping * `ActivityFailure`) — so there is no separate `originalFailure` to retain; - * {@link propagateActivityFailure} re-raises `cause` directly. + * {@link propagateFailure} re-raises `cause` directly. */ export class ActivityCancelledError extends TaggedError(ACTIVITY_CANCELLED_ERROR_TAG, { name: "ActivityCancelledError", diff --git a/packages/worker/src/saga.ts b/packages/worker/src/saga.ts index 1c23cef1..3eb41a36 100644 --- a/packages/worker/src/saga.ts +++ b/packages/worker/src/saga.ts @@ -75,7 +75,7 @@ export type WorkflowSagaBuilder = { * other failure is not. An activity that failed unmodelled, or a child * workflow that did, left state nobody can see, and a defect is a bug rather * than an answer; un-deciding what you cannot see is a second bug, so the - * failure propagates untouched and `propagateActivityFailure` still re-raises + * failure propagates untouched and `propagateFailure` still re-raises * the platform's original failure. * * Cancellation is the one case a caller may opt back in to. diff --git a/packages/worker/src/workflow.ts b/packages/worker/src/workflow.ts index 3e14394c..805b9dc9 100644 --- a/packages/worker/src/workflow.ts +++ b/packages/worker/src/workflow.ts @@ -106,13 +106,22 @@ export { // re-raises the original CancelledFailure so the execution ends `Cancelled`. export { rethrowCancellation } from "./errors.js"; -// Activity-failure re-raise helper: the workflow-side equivalent of "let it -// fail" for any activity call's `AsyncResult` — declared `errors` map or not. -// Re-raises the original Temporal failure (not the -// `ActivityError`/`ActivityCancelledError` wrapper, which isn't a -// `TemporalFailure`) so Temporal classifies the workflow outcome exactly as -// it would have if the activity call still threw directly. -export { propagateActivityFailure } from "./activity-failure.js"; +// The two ways to fold a call's `AsyncResult` when the error channel is not +// worth matching by hand: +// +// - `propagateFailure` — "let Temporal decide". Re-raises the original +// Temporal failure (not the `ActivityError`/`ActivityCancelledError` +// wrapper, which isn't a `TemporalFailure`) so Temporal classifies the +// workflow outcome exactly as it would have if the call still threw +// directly. Covers activity calls, child-workflow calls, and cancellation +// scopes alike. +// - `bestEffort` — "log it and carry on", for a non-critical call. Real +// cancellation is still re-raised, so absorbing a cancel is not something +// each call site has to remember. +// +// `propagateActivityFailure` is the deprecated former name of +// `propagateFailure`. +export { bestEffort, propagateActivityFailure, propagateFailure } from "./activity-failure.js"; // The saga, reachable without a context for the workflow that composes its // steps in a helper. `context.saga` is this same function. @@ -213,7 +222,7 @@ export type { TypedContinueAsNewOptions } from "./internal.js"; * // context.info: WorkflowInfo * * // Every activity call returns an AsyncResult with three channels — - * // narrow `isDefect()`/`isErr()` (or use `propagateActivityFailure` to + * // narrow `isDefect()`/`isErr()` (or use `propagateFailure` to * // let Temporal decide the outcome) before reaching `.value`. * const inventory = await context.activities.validateInventory({ * orderId: args.orderId, @@ -942,7 +951,7 @@ export type WorkflowContext< * `ActivityError`, a `ChildWorkflowError` or a defect: an activity that failed * unmodelled left state nobody can see, and un-deciding what you cannot see is * a second bug. That failure propagates untouched, so - * {@link propagateActivityFailure} still re-raises Temporal's original + * {@link propagateFailure} still re-raises Temporal's original * failure. Cancellation is the one case a caller may opt back in to, with * `saga({ compensateOnCancellation: true })`. Every undo runs inside a * non-cancellable scope, so a cancellation cannot interrupt the walk-back From 05d2bfbb0dd8e12e2381655a280621c55c915bd0 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:02:44 +0200 Subject: [PATCH 02/12] feat(client): ship grouped error patterns for the matcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #421. The order-processing client example imported eleven `_TAG` constants and passed seven `P.tag(...)` arguments to say "anything else failed". Each group here mirrors one method's error union exactly, so spreading it into a `.with()` arm covers that union in one argument. Grouping is typing-only, not checking: the matcher still subtracts each pattern from `Remaining`. The spec pins both directions — `.exhaustive()` compiles for each group over its real union (compile-time), and a `@ts-expect-error` proves the stopped trio still fails to satisfy the wider result union. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .changeset/client-error-patterns.md | 14 ++ packages/client/src/error-patterns.spec.ts | 205 +++++++++++++++++++++ packages/client/src/error-patterns.ts | 138 ++++++++++++++ packages/client/src/index.ts | 13 ++ 4 files changed, 370 insertions(+) create mode 100644 .changeset/client-error-patterns.md create mode 100644 packages/client/src/error-patterns.spec.ts create mode 100644 packages/client/src/error-patterns.ts diff --git a/.changeset/client-error-patterns.md b/.changeset/client-error-patterns.md new file mode 100644 index 00000000..0486daa4 --- /dev/null +++ b/.changeset/client-error-patterns.md @@ -0,0 +1,14 @@ +--- +"@temporal-contract/client": minor +--- + +Ready-made error pattern groups — `WORKFLOW_START_PATTERNS`, +`WORKFLOW_RESULT_PATTERNS`, `WORKFLOW_EXECUTE_PATTERNS`, +`WORKFLOW_STOPPED_PATTERNS`, `SIGNAL_PATTERNS`, `QUERY_PATTERNS`, +`UPDATE_PATTERNS`, `SCHEDULE_CREATE_PATTERNS`. Each mirrors one method's error +union exactly, so `matcher.with(...WORKFLOW_RESULT_PATTERNS, handler)` replaces +six hand-written `P.tag(...)` arguments. + +Exhaustiveness is unchanged: these are ordinary pattern tuples, so a missing +member is still a compile error naming it. Contract errors are deliberately +excluded — match those first with `{ errorName: "..." }`. diff --git a/packages/client/src/error-patterns.spec.ts b/packages/client/src/error-patterns.spec.ts new file mode 100644 index 00000000..51788733 --- /dev/null +++ b/packages/client/src/error-patterns.spec.ts @@ -0,0 +1,205 @@ +/** + * Tests for the shipped pattern groups. + * + * Two things have to hold, and they fail in different ways: + * + * 1. **Each group covers its method's union** — checked at compile time by + * calling `.exhaustive()`, which is typed callable only once the builder's + * `Remaining` is `never`. A group that loses a member stops compiling + * here rather than silently narrowing what callers handle. + * 2. **Each group is no *wider* than its union** — checked at runtime against + * the literal tag list. A group that gains a stray member would still + * compile (an unreachable pattern is legal) while quietly telling readers + * a method can produce something it cannot. + */ +import { match } from "unthrown"; +import { describe, expect, it } from "vitest"; + +import { + QUERY_PATTERNS, + SCHEDULE_CREATE_PATTERNS, + SIGNAL_PATTERNS, + UPDATE_PATTERNS, + WORKFLOW_EXECUTE_PATTERNS, + WORKFLOW_RESULT_PATTERNS, + WORKFLOW_START_PATTERNS, + WORKFLOW_STOPPED_PATTERNS, +} from "./error-patterns.js"; +import type { + QueryFailedError, + QueryValidationError, + ScheduleAlreadyExistsError, + SignalValidationError, + UpdateFailedError, + UpdateRejectedError, + UpdateValidationError, + WorkflowAlreadyStartedError, + WorkflowCancelledError, + WorkflowExecutionNotFoundError, + WorkflowFailedError, + WorkflowNotInContractError, + WorkflowTerminatedError, + WorkflowTimeoutError, + WorkflowValidationError, +} from "./errors.js"; + +// The unions the client's own signatures produce, restated here so a change +// to either side has to be made deliberately on both. +type StartErrors = + | WorkflowNotInContractError + | WorkflowValidationError + | WorkflowAlreadyStartedError; + +type ResultErrors = + | WorkflowValidationError + | WorkflowFailedError + | WorkflowCancelledError + | WorkflowTerminatedError + | WorkflowTimeoutError + | WorkflowExecutionNotFoundError; + +type ExecuteErrors = StartErrors | ResultErrors; +type StoppedErrors = WorkflowCancelledError | WorkflowTerminatedError | WorkflowTimeoutError; +type SignalErrors = SignalValidationError | WorkflowExecutionNotFoundError; +type QueryErrors = QueryValidationError | QueryFailedError | WorkflowExecutionNotFoundError; +type UpdateErrors = + | UpdateValidationError + | UpdateRejectedError + | UpdateFailedError + | WorkflowExecutionNotFoundError; +type ScheduleCreateErrors = + | ScheduleAlreadyExistsError + | WorkflowNotInContractError + | WorkflowValidationError; + +/** + * Compile-time pins. Never invoked — `.exhaustive()` failing to typecheck is + * the assertion, and `tsc` runs over this file. + */ +export function _typeLevelPins(): void { + const start = (error: StartErrors) => + match(error) + .with(...WORKFLOW_START_PATTERNS, () => "handled") + .exhaustive(); + + const result = (error: ResultErrors) => + match(error) + .with(...WORKFLOW_RESULT_PATTERNS, () => "handled") + .exhaustive(); + + const execute = (error: ExecuteErrors) => + match(error) + .with(...WORKFLOW_EXECUTE_PATTERNS, () => "handled") + .exhaustive(); + + const stopped = (error: StoppedErrors) => + match(error) + .with(...WORKFLOW_STOPPED_PATTERNS, () => "handled") + .exhaustive(); + + const signal = (error: SignalErrors) => + match(error) + .with(...SIGNAL_PATTERNS, () => "handled") + .exhaustive(); + + const query = (error: QueryErrors) => + match(error) + .with(...QUERY_PATTERNS, () => "handled") + .exhaustive(); + + const update = (error: UpdateErrors) => + match(error) + .with(...UPDATE_PATTERNS, () => "handled") + .exhaustive(); + + const schedule = (error: ScheduleCreateErrors) => + match(error) + .with(...SCHEDULE_CREATE_PATTERNS, () => "handled") + .exhaustive(); + + // Grouping must not weaken exhaustiveness: the stopped trio is a strict + // subset of the result union, so it must NOT satisfy it. If this stops + // erroring, the groups have stopped being checked at all. + const notExhaustive = (error: ResultErrors) => + match(error) + .with(...WORKFLOW_STOPPED_PATTERNS, () => "handled") + // @ts-expect-error -- WorkflowValidationError/FailedError/ExecutionNotFoundError remain + .exhaustive(); + + void [start, result, execute, stopped, signal, query, update, schedule, notExhaustive]; +} + +const tagsOf = (patterns: readonly { readonly _tag: string }[]) => patterns.map((p) => p._tag); + +describe("client error pattern groups", () => { + it("WORKFLOW_START_PATTERNS names exactly the start-phase errors", () => { + expect(tagsOf(WORKFLOW_START_PATTERNS)).toEqual([ + "@temporal-contract/WorkflowNotInContractError", + "@temporal-contract/WorkflowValidationError", + "@temporal-contract/WorkflowAlreadyStartedError", + ]); + }); + + it("WORKFLOW_RESULT_PATTERNS names exactly the result-phase errors", () => { + expect(tagsOf(WORKFLOW_RESULT_PATTERNS)).toEqual([ + "@temporal-contract/WorkflowValidationError", + "@temporal-contract/WorkflowFailedError", + "@temporal-contract/WorkflowCancelledError", + "@temporal-contract/WorkflowTerminatedError", + "@temporal-contract/WorkflowTimeoutError", + "@temporal-contract/WorkflowExecutionNotFoundError", + ]); + }); + + it("WORKFLOW_EXECUTE_PATTERNS is the union of both phases, without duplicates", () => { + const tags = tagsOf(WORKFLOW_EXECUTE_PATTERNS); + + expect(new Set(tags)).toEqual( + new Set([...tagsOf(WORKFLOW_START_PATTERNS), ...tagsOf(WORKFLOW_RESULT_PATTERNS)]), + ); + expect(tags).toHaveLength(new Set(tags).size); + }); + + it("WORKFLOW_STOPPED_PATTERNS is the stopped trio, a subset of the result phase", () => { + const stopped = tagsOf(WORKFLOW_STOPPED_PATTERNS); + + expect(stopped).toEqual([ + "@temporal-contract/WorkflowCancelledError", + "@temporal-contract/WorkflowTerminatedError", + "@temporal-contract/WorkflowTimeoutError", + ]); + expect(tagsOf(WORKFLOW_RESULT_PATTERNS)).toEqual(expect.arrayContaining(stopped)); + }); + + it("SIGNAL_PATTERNS names exactly what a signal call produces", () => { + expect(tagsOf(SIGNAL_PATTERNS)).toEqual([ + "@temporal-contract/SignalValidationError", + "@temporal-contract/WorkflowExecutionNotFoundError", + ]); + }); + + it("QUERY_PATTERNS names exactly what a query call produces", () => { + expect(tagsOf(QUERY_PATTERNS)).toEqual([ + "@temporal-contract/QueryValidationError", + "@temporal-contract/QueryFailedError", + "@temporal-contract/WorkflowExecutionNotFoundError", + ]); + }); + + it("UPDATE_PATTERNS names exactly what an update call produces", () => { + expect(tagsOf(UPDATE_PATTERNS)).toEqual([ + "@temporal-contract/UpdateValidationError", + "@temporal-contract/UpdateRejectedError", + "@temporal-contract/UpdateFailedError", + "@temporal-contract/WorkflowExecutionNotFoundError", + ]); + }); + + it("SCHEDULE_CREATE_PATTERNS names exactly what schedule.create produces", () => { + expect(tagsOf(SCHEDULE_CREATE_PATTERNS)).toEqual([ + "@temporal-contract/ScheduleAlreadyExistsError", + "@temporal-contract/WorkflowNotInContractError", + "@temporal-contract/WorkflowValidationError", + ]); + }); +}); diff --git a/packages/client/src/error-patterns.ts b/packages/client/src/error-patterns.ts new file mode 100644 index 00000000..0caf4168 --- /dev/null +++ b/packages/client/src/error-patterns.ts @@ -0,0 +1,138 @@ +/** + * Ready-made pattern groups for the client's error channels, so a caller who + * wants "handle my domain error, log the rest" does not hand-write the same + * six `P.tag(...)` arguments at every call site. + * + * Each group mirrors **one method's error union exactly**, so spreading it + * into a `.with(...)` arm covers that union: + * + * ```ts + * import { WORKFLOW_RESULT_PATTERNS } from "@temporal-contract/client"; + * + * (await handle.result()).match({ + * ok: (order) => order.orderId, + * errCases: (matcher) => + * matcher + * .with({ errorName: "PaymentDeclined" }, (err) => err.data.reason) + * .with(...WORKFLOW_RESULT_PATTERNS, (err) => logger.error({ err })), + * defect: (cause) => logger.error({ cause }), + * }); + * ``` + * + * **Exhaustiveness is not weakened.** These are ordinary tuples of ordinary + * patterns: the matcher still subtracts each one from `Remaining`, and a + * member missing from an arm is still a compile error naming it + * (`NonExhaustive`). Grouping saves typing, not + * checking. + * + * **Contract errors are deliberately excluded.** A workflow's declared + * `errors` are user-defined, so no shipped group can name them; match them + * first with the `{ errorName: "..." }` object pattern. For a workflow that + * declares errors, a `WORKFLOW_RESULT_PATTERNS` arm alone is therefore *not* + * exhaustive — which is the correct outcome: a declared domain error deserves + * its own branch. + * + * Kept separate from `error-tags.ts`, which stays free of any `unthrown` + * import so the raw `_tag` constants can be used without pulling in runtime + * machinery. + */ +import { P } from "unthrown"; + +import { + QUERY_FAILED_ERROR_TAG, + QUERY_VALIDATION_ERROR_TAG, + SCHEDULE_ALREADY_EXISTS_ERROR_TAG, + SIGNAL_VALIDATION_ERROR_TAG, + UPDATE_FAILED_ERROR_TAG, + UPDATE_REJECTED_ERROR_TAG, + UPDATE_VALIDATION_ERROR_TAG, + WORKFLOW_ALREADY_STARTED_ERROR_TAG, + WORKFLOW_CANCELLED_ERROR_TAG, + WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG, + WORKFLOW_FAILED_ERROR_TAG, + WORKFLOW_NOT_IN_CONTRACT_ERROR_TAG, + WORKFLOW_TERMINATED_ERROR_TAG, + WORKFLOW_TIMEOUT_ERROR_TAG, + WORKFLOW_VALIDATION_ERROR_TAG, +} from "./error-tags.js"; + +/** + * Every error `ContractClient.startWorkflow` / `signalWithStart` can produce: + * the workflow name is not on the contract, its input failed validation, or + * an execution under this workflow ID already exists. + */ +export const WORKFLOW_START_PATTERNS = [ + P.tag(WORKFLOW_NOT_IN_CONTRACT_ERROR_TAG), + P.tag(WORKFLOW_VALIDATION_ERROR_TAG), + P.tag(WORKFLOW_ALREADY_STARTED_ERROR_TAG), +] as const; + +/** + * The non-contract-error tail of `WorkflowResultErrorsOf` — everything + * `TypedWorkflowHandle.result()` can produce besides the workflow's own + * declared `errors`: output validation, a generic completion failure, the + * three first-class stopped outcomes, and a missing execution. + */ +export const WORKFLOW_RESULT_PATTERNS = [ + P.tag(WORKFLOW_VALIDATION_ERROR_TAG), + P.tag(WORKFLOW_FAILED_ERROR_TAG), + P.tag(WORKFLOW_CANCELLED_ERROR_TAG), + P.tag(WORKFLOW_TERMINATED_ERROR_TAG), + P.tag(WORKFLOW_TIMEOUT_ERROR_TAG), + P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), +] as const; + +/** + * `ContractClient.executeWorkflow` is start + result, so its union is the + * widest: both phases, minus the workflow's own declared `errors`. + */ +export const WORKFLOW_EXECUTE_PATTERNS = [ + P.tag(WORKFLOW_NOT_IN_CONTRACT_ERROR_TAG), + P.tag(WORKFLOW_ALREADY_STARTED_ERROR_TAG), + P.tag(WORKFLOW_VALIDATION_ERROR_TAG), + P.tag(WORKFLOW_FAILED_ERROR_TAG), + P.tag(WORKFLOW_CANCELLED_ERROR_TAG), + P.tag(WORKFLOW_TERMINATED_ERROR_TAG), + P.tag(WORKFLOW_TIMEOUT_ERROR_TAG), + P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), +] as const; + +/** + * The three outcomes that mean "the execution was stopped, and not by + * completing" — cancelled, terminated, timed out. A subset of + * {@link WORKFLOW_RESULT_PATTERNS}, for callers that treat those alike but + * want the remaining failures branched separately. + */ +export const WORKFLOW_STOPPED_PATTERNS = [ + P.tag(WORKFLOW_CANCELLED_ERROR_TAG), + P.tag(WORKFLOW_TERMINATED_ERROR_TAG), + P.tag(WORKFLOW_TIMEOUT_ERROR_TAG), +] as const; + +/** Every error a `handle.signals.*` call can produce. */ +export const SIGNAL_PATTERNS = [ + P.tag(SIGNAL_VALIDATION_ERROR_TAG), + P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), +] as const; + +/** Every error a `handle.queries.*` call can produce. */ +export const QUERY_PATTERNS = [ + P.tag(QUERY_VALIDATION_ERROR_TAG), + P.tag(QUERY_FAILED_ERROR_TAG), + P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), +] as const; + +/** Every error a `handle.updates.*` call can produce. */ +export const UPDATE_PATTERNS = [ + P.tag(UPDATE_VALIDATION_ERROR_TAG), + P.tag(UPDATE_REJECTED_ERROR_TAG), + P.tag(UPDATE_FAILED_ERROR_TAG), + P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), +] as const; + +/** Every error `schedule.create` can produce. */ +export const SCHEDULE_CREATE_PATTERNS = [ + P.tag(SCHEDULE_ALREADY_EXISTS_ERROR_TAG), + P.tag(WORKFLOW_NOT_IN_CONTRACT_ERROR_TAG), + P.tag(WORKFLOW_VALIDATION_ERROR_TAG), +] as const; diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 9c7c24bb..09cd86b1 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -71,6 +71,19 @@ export { WORKFLOW_TIMEOUT_ERROR_TAG, WORKFLOW_VALIDATION_ERROR_TAG, } from "./error-tags.js"; +// Ready-made pattern groups over those tags, each mirroring one method's +// error union — `matcher.with(...WORKFLOW_RESULT_PATTERNS, handler)` instead +// of six hand-written `P.tag(...)` arguments. Exhaustiveness is unchanged. +export { + QUERY_PATTERNS, + SCHEDULE_CREATE_PATTERNS, + SIGNAL_PATTERNS, + UPDATE_PATTERNS, + WORKFLOW_EXECUTE_PATTERNS, + WORKFLOW_RESULT_PATTERNS, + WORKFLOW_START_PATTERNS, + WORKFLOW_STOPPED_PATTERNS, +} from "./error-patterns.js"; export type { ClientInferInput, ClientInferOutput, From 1871c6b06bd52e77ab47b9f2e562ddef5e973c15 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:07:59 +0200 Subject: [PATCH 03/12] feat(contract,worker): declare an activity idempotency key Closes #428. Temporal's at-least-once activity guarantee had no answer in this library: the workflow-level `idempotency` field is start deduplication (it maps to `workflowIdReusePolicy`) and says nothing about an activity running twice. The order-processing example documents the consequence in a 25-line comment about double charges. `defineActivity({ idempotencyKey })` derives the key from the validated input and hands it to the implementation. Payload-derived rather than taken from `Context.current().info`: `activityId` is a per-run command sequence number, so a re-run that branches differently before the call would get a different key. The key is handed over verbatim, with no activity-name prefix, so `runActivity` reproduces exactly what production passes. Typing note: the structural `ActivityDefinition` slot types its parameter `never` (a property-position function type is contravariant, and plain-object contracts using `satisfies ContractDefinition` must still accept a narrowing derivation); `defineActivity` re-states the slot against the bound input schema, which is what gives the lambda its contextual type. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .changeset/activity-idempotency-key.md | 28 +++ packages/contract/src/builder.ts | 17 +- packages/contract/src/types.ts | 45 +++++ packages/testing/src/activity.ts | 7 + .../worker/src/activity-idempotency.spec.ts | 165 ++++++++++++++++++ packages/worker/src/activity.ts | 56 +++++- 6 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 .changeset/activity-idempotency-key.md create mode 100644 packages/worker/src/activity-idempotency.spec.ts diff --git a/.changeset/activity-idempotency-key.md b/.changeset/activity-idempotency-key.md new file mode 100644 index 00000000..387cae0d --- /dev/null +++ b/.changeset/activity-idempotency-key.md @@ -0,0 +1,28 @@ +--- +"@temporal-contract/contract": minor +"@temporal-contract/worker": minor +"@temporal-contract/testing": minor +--- + +Activities can declare an **idempotency key**, derived from their input: + +```ts +const chargeCard = defineActivity({ + input: z.object({ customerId: z.string(), amount: z.number() }), + output: PaymentSchema, + idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, +}); + +chargeCard: ({ input, idempotencyKey }) => + fromPromise(gateway.charge(input, { idempotencyKey }), qualifyFailure("CHARGE_FAILED")), +``` + +Temporal runs activities **at least once**, and nothing in the library helped +with that until now — `idempotency` on a workflow is start deduplication and +says nothing about an activity running twice. Being payload-derived, the key is +stable across activity retries, worker crashes, and a fresh workflow execution +with the same input. + +`helpers.idempotencyKey` is typed `string` for an activity that declares one and +`undefined` for one that does not, so reaching for a key that was never declared +is a compile error. `runActivity` hands over the same value. diff --git a/packages/contract/src/builder.ts b/packages/contract/src/builder.ts index 82ee6082..d2205e69 100644 --- a/packages/contract/src/builder.ts +++ b/packages/contract/src/builder.ts @@ -60,8 +60,21 @@ import type { * }); * ``` */ -export function defineActivity( - definition: TActivity, +export function defineActivity< + TInput extends AnySchema, + TOutput extends AnySchema, + TActivity extends ActivityDefinition, +>( + definition: TActivity & { + readonly input: TInput; + readonly output: TOutput; + /** + * Re-stated against the bound input schema (the structural definition + * types it `never` — see {@link ActivityDefinition}), so this lambda's + * parameter is contextually typed as the activity's validated input. + */ + readonly idempotencyKey?: (input: StandardSchemaV1.InferOutput) => string; + }, ): TActivity { return definition; } diff --git a/packages/contract/src/types.ts b/packages/contract/src/types.ts index 1ca0aaa1..3f1b437c 100644 --- a/packages/contract/src/types.ts +++ b/packages/contract/src/types.ts @@ -133,6 +133,51 @@ export type ActivityDefinition< readonly output: TOutput; readonly errors?: TErrors; readonly activityOptions?: ContractActivityOptions; + /** + * Derive this activity's **idempotency key** from its input. + * + * Temporal runs an activity **at least once**: a retry, a worker crash, or + * a completion that succeeded but was never recorded all re-run the + * implementation. Making the effect idempotent is the application's job, + * and the usual remedy is handing a stable key to the downstream API + * (Stripe's `Idempotency-Key`, and its equivalents). Declaring the key here + * means the caller and the implementation cannot disagree about what it is. + * + * The function receives the **validated** input (post-parse, so schema + * transforms have already run) and must be pure and deterministic: the same + * input has to produce the same key on every attempt, or the key protects + * nothing. + * + * Being derived from the *payload* rather than from Temporal's own + * identifiers is what makes it stable across activity retries, worker + * crashes, **and** a fresh workflow execution started with the same inputs. + * (`Context.current().info.activityId` looks like an alternative and is + * not: it is a per-run command sequence number, so a re-run that branches + * differently before this call gets a different value.) + * + * The parameter is typed `never` **here**, in the structural definition, so + * a contract written as a plain object literal (`satisfies + * ContractDefinition`) still accepts a derivation that narrows its input — + * a property-position function type is contravariant in its parameter, and + * this slot's `TInput` is only known once a concrete schema is bound. + * `defineActivity` re-states the slot against the real input type, so the + * lambda written there is contextually typed and checked. + * + * The key reaches the implementation verbatim. Two activities sharing a + * downstream keyspace must therefore disambiguate in their own derivations + * (`` `charge:${orderId}` `` vs `` `refund:${orderId}` ``) — handing a + * gateway one key for two opposite operations is the failure to avoid. + * + * @example + * ```ts + * const chargeCard = defineActivity({ + * input: z.object({ customerId: z.string(), amount: z.number() }), + * output: PaymentSchema, + * idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, + * }); + * ``` + */ + readonly idempotencyKey?: (input: never) => string; }; /** diff --git a/packages/testing/src/activity.ts b/packages/testing/src/activity.ts index 5d96a1ef..7b2451fd 100644 --- a/packages/testing/src/activity.ts +++ b/packages/testing/src/activity.ts @@ -134,6 +134,13 @@ export function runActivity, context: {}, input: options.input, + // Same value production hands over: the declared derivation applied to + // this input, verbatim. The structural slot types its parameter `never` + // so plain-object contracts stay assignable (see `ActivityDefinition`); + // the value passed here is the input the derivation was written against. + idempotencyKey: (definition.idempotencyKey as ((input: unknown) => string) | undefined)?.( + options.input, + ), }; return _internal_makeAsyncResult(() => diff --git a/packages/worker/src/activity-idempotency.spec.ts b/packages/worker/src/activity-idempotency.spec.ts new file mode 100644 index 00000000..2da29e96 --- /dev/null +++ b/packages/worker/src/activity-idempotency.spec.ts @@ -0,0 +1,165 @@ +/** + * The activity idempotency key: declared on the contract, derived from the + * validated input, handed to the implementation. + * + * Temporal runs an activity at least once, so the value these tests pin is + * the one a payment gateway will see on the second run. What matters is that + * it is **identical** across runs of the same input, and that it is derived + * from the input the implementation actually receives. + */ +import { defineActivity, type ContractDefinition } from "@temporal-contract/contract"; +import { OkAsync } from "unthrown"; +import { describe, expect, expectTypeOf, it } from "vitest"; +import { z } from "zod"; + +import { + declareActivitiesHandler, + declareActivityMiddleware, + type ActivityImplementationHelpers, +} from "./activity.js"; + +const contract = { + taskQueue: "payments", + workflows: { + checkout: { + input: z.object({ orderId: z.string() }), + output: z.object({ done: z.boolean() }), + idempotency: "retry-if-failed", + }, + }, + activities: { + // Declares a key: the customer + amount pair a gateway must not charge twice. + charge: { + input: z.object({ customerId: z.string(), amount: z.number() }), + output: z.object({ key: z.string() }), + idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, + }, + // Declares none: reading a balance twice is harmless. + readBalance: { + input: z.object({ customerId: z.string() }), + output: z.object({ key: z.string() }), + }, + // The input schema transforms, so the key must be derived AFTER the parse. + chargeTrimmed: { + input: z.object({ customerId: z.string().transform((v) => v.trim()) }), + output: z.object({ key: z.string() }), + idempotencyKey: ({ customerId }) => customerId, + }, + }, +} satisfies ContractDefinition; + +/** Hands the received key straight back so a test can assert on it. */ +const echoKey = ({ idempotencyKey }: { idempotencyKey: string | undefined }) => + OkAsync({ key: String(idempotencyKey) }); + +describe("activity idempotency key", () => { + it("hands the declared key, derived from the input, to the implementation", async () => { + const activities = declareActivitiesHandler({ + contract, + activities: { + charge: echoKey, + readBalance: echoKey, + chargeTrimmed: echoKey, + }, + }); + + await expect(activities.charge({ customerId: "CUST-1", amount: 149.97 })).resolves.toEqual({ + key: "CUST-1:149.97", + }); + }); + + it("produces the SAME key on a re-run of the same input", async () => { + // The at-least-once guarantee in one assertion: two invocations, one key. + const activities = declareActivitiesHandler({ + contract, + activities: { charge: echoKey, readBalance: echoKey, chargeTrimmed: echoKey }, + }); + + const first = await activities.charge({ customerId: "CUST-1", amount: 149.97 }); + const second = await activities.charge({ customerId: "CUST-1", amount: 149.97 }); + + expect(first).toEqual(second); + }); + + it("hands over undefined when the activity declares no key", async () => { + const activities = declareActivitiesHandler({ + contract, + activities: { charge: echoKey, readBalance: echoKey, chargeTrimmed: echoKey }, + }); + + await expect(activities.readBalance({ customerId: "CUST-1" })).resolves.toEqual({ + key: "undefined", + }); + }); + + it("derives from the VALIDATED input, after schema transforms", async () => { + // Deriving from the raw payload would key " CUST-1 " and "CUST-1" + // differently — two keys for one customer, and a double charge. + const activities = declareActivitiesHandler({ + contract, + activities: { charge: echoKey, readBalance: echoKey, chargeTrimmed: echoKey }, + }); + + await expect(activities.chargeTrimmed({ customerId: " CUST-1 " })).resolves.toEqual({ + key: "CUST-1", + }); + }); + + it("re-keys on a middleware input substitution", async () => { + // Middleware may replace the input (re-validated at the boundary); the + // key must describe what actually ran, not what the caller sent. + const rewrite = declareActivityMiddleware(({ input }, next) => { + const typed = input as { customerId: string; amount: number }; + return next({ input: { ...typed, customerId: "CUST-REWRITTEN" } }); + }); + + const activities = declareActivitiesHandler({ + contract, + middleware: rewrite, + activities: { charge: echoKey, readBalance: echoKey, chargeTrimmed: echoKey }, + }); + + await expect(activities.charge({ customerId: "CUST-1", amount: 10 })).resolves.toEqual({ + key: "CUST-REWRITTEN:10", + }); + }); +}); + +describe("activity idempotency key — types", () => { + // `defineActivity` re-states the slot against the bound input schema, so the + // derivation's parameter is contextually typed: no annotation needed, and a + // field the input doesn't have is a compile error. + const charge = defineActivity({ + input: z.object({ customerId: z.string(), amount: z.number() }), + output: z.object({ ok: z.boolean() }), + idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, + }); + + const readBalance = defineActivity({ + input: z.object({ customerId: z.string() }), + output: z.object({ ok: z.boolean() }), + }); + + it("types the helper as string when the activity declares a key", () => { + expectTypeOf< + ActivityImplementationHelpers["idempotencyKey"] + >().toEqualTypeOf(); + }); + + it("types the helper as undefined when it does not", () => { + // Not `string | undefined`: reaching for a key that was never declared is + // a type error, rather than an `undefined` reaching a payment gateway. + expectTypeOf< + ActivityImplementationHelpers["idempotencyKey"] + >().toEqualTypeOf(); + }); + + it("rejects a derivation reading a field the input does not have", () => { + defineActivity({ + input: z.object({ customerId: z.string() }), + output: z.object({ ok: z.boolean() }), + // @ts-expect-error -- `amount` is not on this activity's input + idempotencyKey: ({ amount }) => String(amount), + }); + }); +}); diff --git a/packages/worker/src/activity.ts b/packages/worker/src/activity.ts index 9420b7e0..f329b45c 100644 --- a/packages/worker/src/activity.ts +++ b/packages/worker/src/activity.ts @@ -275,8 +275,27 @@ export type ActivityImplementationHelpers< readonly errors: ActivityErrorConstructorsOf; readonly context: TContext; readonly input: WorkerInferInput; + /** + * The activity's idempotency key for this invocation — `string` when the + * contract declares `idempotencyKey`, and `undefined` when it does not, so + * reaching for a key that was never declared is a type error rather than a + * silent `undefined` reaching a payment gateway. + * + * Derived from the validated input, verbatim. Stable across retries of + * this activity, across worker crashes, and across a fresh workflow + * execution with the same input — see `idempotencyKey` on the contract's + * `defineActivity`. + */ + readonly idempotencyKey: ActivityIdempotencyKeyOf; }; +/** + * `string` for an activity that declares `idempotencyKey`, `undefined` for + * one that does not. + */ +export type ActivityIdempotencyKeyOf = + TActivity["idempotencyKey"] extends (input: never) => string ? string : undefined; + /** * Activity implementation using unthrown's `AsyncResult`. * @@ -888,6 +907,28 @@ export function declareActivitiesHandler< // Prepare Temporal-compatible activities with validation and Result unwrapping const wrappedActivities = {} as ActivitiesHandler; + /** + * The declared idempotency key for one invocation, or `undefined` when the + * activity declares none. + * + * Handed over **verbatim** — no activity-name prefix. Prefixing would make + * the key the implementation sees differ from the one the derivation + * function returns, and `runActivity` (which has a definition but no runtime + * activity name) could not reproduce it, so a unit test would exercise a + * different key than production. An activity sharing a downstream keyspace + * with another should say so in its own derivation: `` `charge:${orderId}` ``. + */ + function deriveIdempotencyKey( + activityDef: ActivityDefinition, + input: unknown, + ): string | undefined { + // The structural slot types its parameter `never` so plain-object contracts + // stay assignable (see `ActivityDefinition`); the value passed here is the + // validated input the derivation was written against. + const derive = activityDef.idempotencyKey as ((input: unknown) => string) | undefined; + return derive?.(input); + } + // Helper to create a wrapped implementation from a definition and impl. // `label` is the diagnostic name used in validation errors (workflow-local // activities keep the historical `workflow.activity` format); `info` is the @@ -897,7 +938,7 @@ export function declareActivitiesHandler< info: ActivityInvocationInfo, activityDef: ActivityDefinition, activityImpl: ( - helpers: { errors: unknown; context: unknown; input: unknown }, + helpers: { errors: unknown; context: unknown; input: unknown; idempotencyKey: unknown }, args: unknown, ) => AsyncResult, ) { @@ -930,7 +971,16 @@ export function declareActivitiesHandler< stageContext: Record, ): AsyncResult => activityImpl( - { errors: errorConstructors, context: stageContext, input: stageInput }, + { + errors: errorConstructors, + context: stageContext, + input: stageInput, + // Derived from the input the implementation is about to see — + // `stageInput`, not the caller's original — so a middleware + // `next({ input })` substitution (already re-validated above) + // keys the downstream call on what actually ran. + idempotencyKey: deriveIdempotencyKey(activityDef, stageInput), + }, stageInput, ); @@ -1024,7 +1074,7 @@ export function declareActivitiesHandler< } type ErasedImplementation = ( - helpers: { errors: unknown; context: unknown; input: unknown }, + helpers: { errors: unknown; context: unknown; input: unknown; idempotencyKey: unknown }, args: unknown, ) => AsyncResult; From f7e1b144739dfd4f2eeb7f062488671003962874 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:12:31 +0200 Subject: [PATCH 04/12] feat(contract,client)!: derive the workflow ID from the payload Closes #429. `startPolicy` declared the policy while the caller supplied the key it acts on, so `client.startWorkflow(name, { workflowId: crypto.randomUUID(), ... })` compiled and made `"once-per-id"` inert with no diagnostic. A workflow that declares `workflowId` now derives it from the validated input, and `TypedWorkflowStartOptions` types the field as `never` for those workflows, so the two cannot disagree. Derivation runs on the post-parse value (the client already validated it; the caller's original still crosses the wire under D1), so a schema transform can't give one logical request two IDs. `IdempotencyMode` becomes `WorkflowStartPolicy`, with the old name kept as a deprecated alias. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .changeset/derived-workflow-id.md | 25 +++ docs/explanation/why-temporal-contract.md | 2 +- docs/how-to/continue-as-new.md | 2 +- docs/how-to/define-a-contract.md | 10 +- .../index-workflows-with-search-attributes.md | 4 +- docs/how-to/install.md | 2 +- docs/how-to/model-domain-errors.md | 2 +- docs/how-to/run-child-workflows.md | 2 +- docs/how-to/schedule-workflows.md | 6 +- docs/how-to/upgrade-to-v8.md | 59 ++++++- .../how-to/use-signals-queries-and-updates.md | 2 +- docs/index.md | 2 +- docs/reference/contract-surface.md | 4 +- docs/tutorial/adding-signals-and-queries.md | 2 +- docs/tutorial/your-first-workflow.md | 4 +- .../order-processing-contract/src/contract.ts | 6 +- .../client/src/__tests__/second.contract.ts | 2 +- .../client/src/__tests__/test.contract.ts | 8 +- packages/client/src/client.spec.ts | 34 ++-- packages/client/src/client.ts | 70 ++++++-- packages/client/src/schedule.spec.ts | 6 +- packages/client/src/types-inference.spec.ts | 6 +- packages/client/src/workflow-id.spec.ts | 149 ++++++++++++++++++ packages/contract/src/builder.spec.ts | 136 ++++++++-------- packages/contract/src/builder.ts | 39 +++-- packages/contract/src/helpers.spec.ts | 4 +- packages/contract/src/idempotency.ts | 16 +- packages/contract/src/internal.ts | 4 +- packages/contract/src/types-inference.spec.ts | 28 ++-- packages/contract/src/types.spec.ts | 18 +-- packages/contract/src/types.ts | 44 +++++- .../testing/src/__tests__/test.contract.ts | 2 +- packages/testing/src/workflow-bundle.spec.ts | 2 +- .../__tests__/activity-options.contract.ts | 4 +- .../src/__tests__/cancellation.contract.ts | 8 +- .../__tests__/child-idempotency.contract.ts | 4 +- .../child-idempotency.inprocess.spec.ts | 2 +- .../src/__tests__/child-wire.contract.ts | 12 +- .../src/__tests__/continue-as-new.contract.ts | 12 +- .../worker/src/__tests__/handlers.contract.ts | 12 +- .../src/__tests__/idempotency.contract.ts | 6 +- .../src/__tests__/inprocess.contract.ts | 4 +- .../src/__tests__/propagation.contract.ts | 4 +- .../src/__tests__/registration.contract.ts | 4 +- .../src/__tests__/rehydration.contract.ts | 4 +- .../worker/src/__tests__/retry.contract.ts | 2 +- .../worker/src/__tests__/routing.contract.ts | 2 +- .../worker/src/__tests__/saga.contract.ts | 4 +- .../worker/src/__tests__/test.contract.ts | 12 +- .../worker/src/__tests__/timeouts.contract.ts | 2 +- .../src/activity-contract-errors.spec.ts | 4 +- .../worker/src/activity-idempotency.spec.ts | 2 +- packages/worker/src/activity.spec.ts | 16 +- packages/worker/src/child-workflow.ts | 10 +- packages/worker/src/handlers.spec.ts | 2 +- packages/worker/src/types-inference.spec.ts | 4 +- packages/worker/src/workflow-options.spec.ts | 4 +- packages/worker/src/workflow.spec.ts | 4 +- packages/worker/src/workflow.ts | 4 +- 59 files changed, 588 insertions(+), 262 deletions(-) create mode 100644 .changeset/derived-workflow-id.md create mode 100644 packages/client/src/workflow-id.spec.ts diff --git a/.changeset/derived-workflow-id.md b/.changeset/derived-workflow-id.md new file mode 100644 index 00000000..d80652a7 --- /dev/null +++ b/.changeset/derived-workflow-id.md @@ -0,0 +1,25 @@ +--- +"@temporal-contract/contract": minor +"@temporal-contract/client": minor +--- + +Workflows can derive their **workflow ID** from their input: + +```ts +const processOrder = defineWorkflow({ + input: OrderSchema, + output: OrderResultSchema, + workflowId: ({ orderId }) => `order-${orderId}`, + startPolicy: "once-per-id", +}); +``` + +`startPolicy` only bites when two starts of the same logical request collide on +one ID, and the ID used to be entirely the caller's — passing +`crypto.randomUUID()` made `"once-per-id"` inert with no diagnostic. A workflow +that declares `workflowId` now derives it from the validated payload on +`startWorkflow` / `executeWorkflow` / `signalWithStart`, and supplying one at +the call site is a type error. Workflows that declare none are unchanged. + +`IdempotencyMode` is renamed to `WorkflowStartPolicy` (the old name stays as a +deprecated type alias). diff --git a/docs/explanation/why-temporal-contract.md b/docs/explanation/why-temporal-contract.md index aefe798b..2ba59020 100644 --- a/docs/explanation/why-temporal-contract.md +++ b/docs/explanation/why-temporal-contract.md @@ -68,7 +68,7 @@ const processOrder = defineWorkflow({ amount: z.number().positive(), }), output: z.object({ transactionId: z.string() }), - idempotency: "retry-if-failed", // charges a card — see Define a contract + startPolicy: "retry-if-failed", // charges a card — see Define a contract activities: { chargeCard }, }); diff --git a/docs/how-to/continue-as-new.md b/docs/how-to/continue-as-new.md index aad1fd42..751286f9 100644 --- a/docs/how-to/continue-as-new.md +++ b/docs/how-to/continue-as-new.md @@ -101,7 +101,7 @@ const pollSubscription = defineWorkflow({ // fine. It does NOT guard against double-charging a cycle: that's // `chargeSubscription`'s job (an idempotency key derived from // `lastChargeId`/`cycle`), independent of this field. - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); ``` diff --git a/docs/how-to/define-a-contract.md b/docs/how-to/define-a-contract.md index 49de2499..d65d6ff9 100644 --- a/docs/how-to/define-a-contract.md +++ b/docs/how-to/define-a-contract.md @@ -21,7 +21,7 @@ const chargeCard = defineActivity({ const processOrder = defineWorkflow({ input: z.object({ orderId: z.string(), customerId: z.string() }), output: z.object({ status: z.enum(["completed", "failed"]) }), - idempotency: "retry-if-failed", // charges a card — see below + startPolicy: "retry-if-failed", // charges a card — see below activities: { chargeCard }, }); @@ -38,7 +38,7 @@ contents. ## Declare idempotency -`idempotency` is required on every workflow. It answers one question: **is it +`startPolicy` is required on every workflow. It answers one question: **is it safe to start this workflow ID again after a previous run has closed?** Temporal's own default (`workflowIdReusePolicy: ALLOW_DUPLICATE`) says yes — @@ -66,7 +66,7 @@ const chargeOrder = defineWorkflow({ // successful run under the same order ID. A start is still retryable // after a genuinely failed attempt (e.g. a declined payment, where no // charge went through). - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", activities: { chargeCard }, }); ``` @@ -197,7 +197,7 @@ const changeAddress = defineUpdate({ const processOrder = defineWorkflow({ input: OrderSchema, output: OrderResultSchema, - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", activities: { chargeCard }, signals: { approve }, queries: { getStatus }, @@ -250,7 +250,7 @@ import { defineSearchAttribute } from "@temporal-contract/contract"; const processOrder = defineWorkflow({ input: OrderSchema, output: OrderResultSchema, - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", searchAttributes: { customerId: defineSearchAttribute({ kind: "KEYWORD" }), orderTotal: defineSearchAttribute({ kind: "DOUBLE" }), diff --git a/docs/how-to/index-workflows-with-search-attributes.md b/docs/how-to/index-workflows-with-search-attributes.md index fbc186b2..9a90e9aa 100644 --- a/docs/how-to/index-workflows-with-search-attributes.md +++ b/docs/how-to/index-workflows-with-search-attributes.md @@ -15,11 +15,11 @@ import { z } from "zod"; export const processOrder = defineWorkflow({ input: OrderSchema, output: OrderResultSchema, - // Search attributes are this doc's topic, but `idempotency` is required on + // Search attributes are this doc's topic, but `startPolicy` is required on // every workflow. `retry-if-failed` is the right mode for this shape — an // order that charged a customer must not be re-runnable after a Completed // run. See "Declare idempotency" in define-a-contract.md. - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", searchAttributes: { customerId: defineSearchAttribute({ kind: "KEYWORD" }), orderTotal: defineSearchAttribute({ kind: "DOUBLE" }), diff --git a/docs/how-to/install.md b/docs/how-to/install.md index 0428aa0d..59f93236 100644 --- a/docs/how-to/install.md +++ b/docs/how-to/install.md @@ -224,7 +224,7 @@ import { z } from "zod"; const ping = defineWorkflow({ input: z.object({ message: z.string() }), output: z.object({ echo: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); const contract = defineContract({ diff --git a/docs/how-to/model-domain-errors.md b/docs/how-to/model-domain-errors.md index 26360532..938c3be1 100644 --- a/docs/how-to/model-domain-errors.md +++ b/docs/how-to/model-domain-errors.md @@ -88,7 +88,7 @@ Workflow errors are **thrown**, not returned: const processOrder = defineWorkflow({ input: OrderSchema, output: OrderResultSchema, - idempotency: "retry-if-failed", // charges a card + startPolicy: "retry-if-failed", // charges a card errors: { EmptyOrder: { data: z.object({ orderId: z.string() }), diff --git a/docs/how-to/run-child-workflows.md b/docs/how-to/run-child-workflows.md index b5cb780e..7da67461 100644 --- a/docs/how-to/run-child-workflows.md +++ b/docs/how-to/run-child-workflows.md @@ -208,7 +208,7 @@ await context.executeChildWorkflow(orderContract, "collectPayment", { retry: { maximumAttempts: 3 }, // Reuse behaviour when the id already exists. The child's contract already - // supplies this from its `idempotency` mode — set it here only to override + // supplies this from its `startPolicy` mode — set it here only to override // that default for this one call. workflowIdReusePolicy: "ALLOW_DUPLICATE_FAILED_ONLY", }); diff --git a/docs/how-to/schedule-workflows.md b/docs/how-to/schedule-workflows.md index b14cf9bf..73e5ae5b 100644 --- a/docs/how-to/schedule-workflows.md +++ b/docs/how-to/schedule-workflows.md @@ -119,7 +119,7 @@ await ledger.schedule `overlap: "SKIP"` is the safe default for anything non-idempotent. `ALLOW_ALL` will happily run twenty copies at once after an outage. -Note that a contract's `idempotency` mode does not help here either way: it +Note that a contract's `startPolicy` mode does not help here either way: it governs `workflowIdReusePolicy` for a _new_ run under a workflow ID that a _previous, already-closed_ run held, not overlap between a scheduled run and one still in flight. `overlap` is the only lever for that on this path — see @@ -173,9 +173,9 @@ are nested separately. `workflowType` and `taskQueue` are owned by the contract and are not settable. -::: warning `action.workflowId` bypasses the contract's `idempotency` mode +::: warning `action.workflowId` bypasses the contract's `startPolicy` mode Pinning a fixed `action.workflowId` here does **not** get the protection of -the workflow's declared `idempotency` mode. `schedule.create` builds a plain +the workflow's declared `startPolicy` mode. `schedule.create` builds a plain `ScheduleOptionsStartWorkflowAction`, which has no `workflowIdReusePolicy` field — every scheduled run is started with Temporal's own default (`ALLOW_DUPLICATE`), regardless of whether the contract says `once-per-id`, diff --git a/docs/how-to/upgrade-to-v8.md b/docs/how-to/upgrade-to-v8.md index ce0ae394..86850591 100644 --- a/docs/how-to/upgrade-to-v8.md +++ b/docs/how-to/upgrade-to-v8.md @@ -874,9 +874,9 @@ contract-error wire round-trip) so a test fails exactly where production does. - `@temporalio/*` peer ranges tightened to `^1.16.0` (the real floor for the Schedule API and the search-attribute imports). -## 13. Workflows must declare `idempotency` +## 13. Workflows must declare `startPolicy` -Every `defineWorkflow` now takes a required `idempotency` field. This is a +Every `defineWorkflow` now takes a required `startPolicy` field. This is a breaking change every consumer hits — there is no default to inherit. Temporal's `workflowIdReusePolicy` defaults to `ALLOW_DUPLICATE`, which @@ -884,7 +884,7 @@ permits starting a new run under a workflow ID whose previous run reached **any** Closed state — including Completed. For a workflow keyed `charge-${orderId}`, a client that retries a start after, say, a network timeout — not knowing the first attempt actually went through — starts a -**second** charge under the same order ID. `idempotency` makes the answer to +**second** charge under the same order ID. `startPolicy` makes the answer to "is this safe?" part of the workflow's own definition instead of something every call site has to get right on its own: @@ -892,7 +892,7 @@ every call site has to get right on its own: defineWorkflow({ input, output, - idempotency: "retry-if-failed", // re-runnable only if the last attempt didn't succeed + startPolicy: "retry-if-failed", // re-runnable only if the last attempt didn't succeed }); ``` @@ -916,13 +916,13 @@ does not. that is exactly Temporal's pre-8.0 default, reproduced faithfully. The field is required specifically so that choice is made once, deliberately, per workflow, rather than inherited silently; treat a sweep of -`idempotency: "allow-duplicate"` as a placeholder to revisit workflow by +`startPolicy: "allow-duplicate"` as a placeholder to revisit workflow by workflow, not as the final answer. ::: warning TypeScript enforces the field; a plain JavaScript caller does not get an error -Omitting `idempotency` is a compile error under TypeScript — `WorkflowDefinition` +Omitting `startPolicy` is a compile error under TypeScript — `WorkflowDefinition` requires it. At runtime, though, `defineContract`'s validator deliberately -still accepts a definition with `idempotency` **missing** (as opposed to +still accepts a definition with `startPolicy` **missing** (as opposed to present-but-misspelled, which still throws) — this is what keeps an already-compiled artifact, or a contract assembled outside the type system, from failing validation. A plain-JS caller who skips the field gets no error @@ -939,6 +939,49 @@ worker-initiated child-workflow starts each have a dedicated integration suite that starts real executions and checks which ones the server actually accepts or rejects. +### Let the contract derive the workflow ID + +`startPolicy` only bites if two starts of the same logical request actually +collide on one workflow ID — and until now the ID was entirely the caller's: + +```typescript +// compiles, and silently makes `once-per-id` inert: every start is a fresh ID +client.startWorkflow("processOrder", { workflowId: crypto.randomUUID(), args: order }); +``` + +Declare `workflowId` on the workflow and the ID moves into the contract. +Passing one at the call site then becomes a **type error**, so the policy and +the thing it keys on can no longer disagree: + +```typescript +const processOrder = defineWorkflow({ + input: OrderSchema, + output: OrderResultSchema, + workflowId: ({ orderId }) => `order-${orderId}`, + startPolicy: "once-per-id", +}); + +// ID derived from the payload — no `workflowId` accepted here +await client.startWorkflow("processOrder", { args: order }); +``` + +The derivation runs against the **validated** input (post-parse, so schema +transforms have already applied) and must be pure. It is optional: a workflow +that declares none keeps requiring `workflowId` from the caller, exactly as +before. + +Not applied to `schedule.create`, which generates one ID per firing — a +scheduled run wants a distinct execution, not deduplication. + +### `IdempotencyMode` is now `WorkflowStartPolicy` + +The type behind the field follows the field's own rename. `IdempotencyMode` +remains as a deprecated alias. The name matters because the old one invited a +real mistake: this governs `workflowIdReusePolicy` — whether a workflow ID may +be reused after a Closed run — and it does **not** make a workflow idempotent. +For an activity re-running under Temporal's at-least-once guarantee, see +[an activity's `idempotencyKey`](/how-to/implement-activities). + ## 14. Activity bounds and required `parentClosePolicy` Two more safety requirements are enforced instead of assumed. Both are @@ -1143,7 +1186,7 @@ names the input. registered — a data-less contract error from a 7.x worker carries no wire marker and degrades to a generic failure until the workers are cut over - [ ] `@temporalio/*` resolve to `^1.16.0`; no CJS `require` of these packages -- [ ] Every `defineWorkflow` declares `idempotency`; a migration wanting zero +- [ ] Every `defineWorkflow` declares `startPolicy`; a migration wanting zero behavior change uses `"allow-duplicate"` everywhere, then revisits each workflow deliberately — remember plain-JS (non-type-checked) callers get no runtime error for an omitted field, only for a misspelled one diff --git a/docs/how-to/use-signals-queries-and-updates.md b/docs/how-to/use-signals-queries-and-updates.md index a49bddf6..ef193067 100644 --- a/docs/how-to/use-signals-queries-and-updates.md +++ b/docs/how-to/use-signals-queries-and-updates.md @@ -51,7 +51,7 @@ const addItems = defineUpdate({ const importCatalog = defineWorkflow({ input: z.object({ catalogId: z.string() }), output: z.object({ imported: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { listSkus, importSku }, queries: { getProgress }, signals: { cancelRequested }, diff --git a/docs/index.md b/docs/index.md index 9ad52ef9..95426274 100644 --- a/docs/index.md +++ b/docs/index.md @@ -59,7 +59,7 @@ const processOrder = defineWorkflow({ // Payment already moved money on success — block a second successful // run per order. A start is still retryable after a genuinely failed // attempt (e.g. a declined payment, where no charge went through). - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", activities: { chargeCard }, }); diff --git a/docs/reference/contract-surface.md b/docs/reference/contract-surface.md index 11615fce..cd0ecfd9 100644 --- a/docs/reference/contract-surface.md +++ b/docs/reference/contract-surface.md @@ -61,7 +61,7 @@ relaxes when the contract exists purely to serve activities. | ------------------ | ------------------------------------------- | -------- | | `input` | `AnySchema` | yes | | `output` | `AnySchema` | yes | -| `idempotency` | `IdempotencyMode` | yes | +| `startPolicy` | `IdempotencyMode` | yes | | `activities` | `Record` | no | | `signals` | `Record` | no | | `queries` | `Record` | no | @@ -69,7 +69,7 @@ relaxes when the contract exists purely to serve activities. | `searchAttributes` | `Record` | no | | `errors` | `Record` | no | -`idempotency` governs what happens when this workflow ID is started again +`startPolicy` governs what happens when this workflow ID is started again after a previous run has **closed** — `"once-per-id"` (`REJECT_DUPLICATE`), `"retry-if-failed"` (`ALLOW_DUPLICATE_FAILED_ONLY` — re-runnable after any Closed state other than Completed: Failed, Cancelled, Terminated, or diff --git a/docs/tutorial/adding-signals-and-queries.md b/docs/tutorial/adding-signals-and-queries.md index 759c0266..c7d833c1 100644 --- a/docs/tutorial/adding-signals-and-queries.md +++ b/docs/tutorial/adding-signals-and-queries.md @@ -79,7 +79,7 @@ const processOrder = defineWorkflow({ orderId: z.string(), transactionId: z.string(), }), - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", activities: { chargeCard, sendReceipt }, queries: { getStatus }, signals: { approve }, diff --git a/docs/tutorial/your-first-workflow.md b/docs/tutorial/your-first-workflow.md index d0bd3454..2cd76e84 100644 --- a/docs/tutorial/your-first-workflow.md +++ b/docs/tutorial/your-first-workflow.md @@ -119,7 +119,7 @@ const processOrder = defineWorkflow({ // `once-per-id` would close that gap at the cost of a fresh order ID for // every retry — kept as `retry-if-failed` here to keep this first // tutorial's failure story to one activity (see Step 7). - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", // Activities declared here are reachable only from this workflow. activities: { chargeCard, sendReceipt }, }); @@ -140,7 +140,7 @@ Three things to notice: Valibot and ArkType are equally valid. - `taskQueue` lives on the contract, so neither the worker nor the client has to repeat it. -- `idempotency` is required on every workflow — it is what stops a retried +- `startPolicy` is required on every workflow — it is what stops a retried start from re-running a workflow that already finished. See [Define a contract](/how-to/define-a-contract#declare-idempotency) for the three modes and why the field exists. diff --git a/examples/order-processing-contract/src/contract.ts b/examples/order-processing-contract/src/contract.ts index c08109a0..0bbdf42a 100644 --- a/examples/order-processing-contract/src/contract.ts +++ b/examples/order-processing-contract/src/contract.ts @@ -188,7 +188,7 @@ const processOrder = defineWorkflow({ // legitimate retry, including the common pre-charge `PaymentDeclined` case // above — kept as `retry-if-failed` here because that trade favors the // common case, not because the gap doesn't exist. - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", activities: { processPayment, reserveInventory, @@ -221,12 +221,12 @@ const cleanupExpiredOrders = defineWorkflow({ // declaration is inert here regardless of which mode is picked: // `schedule.create`'s action type has no `workflowIdReusePolicy` field, so // every scheduled run gets Temporal's own default (`ALLOW_DUPLICATE`) no - // matter what `idempotency` says (see "Schedule workflows" in the docs). + // matter what `startPolicy` says (see "Schedule workflows" in the docs). // `allow-duplicate` is chosen anyway to document the intent for any path // that *does* apply it — a direct `client.startWorkflow` under this // workflow's type, for instance — and because purging expired orders // twice is harmless. - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); // ============================================================================ diff --git a/packages/client/src/__tests__/second.contract.ts b/packages/client/src/__tests__/second.contract.ts index e025dbaa..e4222847 100644 --- a/packages/client/src/__tests__/second.contract.ts +++ b/packages/client/src/__tests__/second.contract.ts @@ -17,7 +17,7 @@ export const secondContract = defineContract({ output: z.object({ echoed: z.string(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); diff --git a/packages/client/src/__tests__/test.contract.ts b/packages/client/src/__tests__/test.contract.ts index bf4e31b7..aea93021 100644 --- a/packages/client/src/__tests__/test.contract.ts +++ b/packages/client/src/__tests__/test.contract.ts @@ -23,7 +23,7 @@ export const testContract = defineContract({ output: z.object({ result: z.string(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), // Workflow with signals, queries, and updates @@ -34,7 +34,7 @@ export const testContract = defineContract({ output: z.object({ finalValue: z.number(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { increment: defineSignal({ input: z.object({ @@ -76,7 +76,7 @@ export const testContract = defineContract({ // receive-side parse doubles it. doubled: z.number().transform((n) => n * 2), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), // Workflow with activities @@ -87,7 +87,7 @@ export const testContract = defineContract({ output: z.object({ result: z.string(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { processMessage: defineActivity({ input: z.object({ diff --git a/packages/client/src/client.spec.ts b/packages/client/src/client.spec.ts index b22ca5fe..08316418 100644 --- a/packages/client/src/client.spec.ts +++ b/packages/client/src/client.spec.ts @@ -193,7 +193,7 @@ describe("TypedClient", () => { testWorkflow: { input: z.object({ name: z.string(), value: z.number() }), output: z.object({ result: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { getStatus: { input: z.tuple([]), @@ -215,7 +215,7 @@ describe("TypedClient", () => { simpleWorkflow: { input: z.object({ message: z.string() }), output: z.string(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -294,7 +294,7 @@ describe("TypedClient", () => { otherWorkflow: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({ ok: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -1015,7 +1015,7 @@ describe("TypedClient", () => { processOrder: defineWorkflow({ input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { cancel: { input: z.tuple([z.object({ reason: z.string() })]) }, }, @@ -1030,7 +1030,7 @@ describe("TypedClient", () => { plain: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -1590,7 +1590,7 @@ describe("TypedClient — wire format (validate on send, parse on receive)", () // Asymmetric transform: input type is `string`, parsed type is `number`. input: z.string().transform((s) => s.length), output: z.number().transform((n) => n * 2), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { ping: { input: z.string().transform((s) => s.length) }, }, @@ -1783,7 +1783,7 @@ describe("TypedClient — workflow contract errors", () => { processOrder: defineWorkflow({ input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", errors: { EmptyOrder: { data: z.object({ orderId: z.string() }), @@ -1914,7 +1914,7 @@ describe("ContractClient — handle identifiers and validation-error identity", identityWorkflow: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({ ok: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -2039,7 +2039,7 @@ describe("ContractClient — startUpdate", () => { updatable: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({ ok: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", updates: { adjust: { input: z.object({ delta: z.number() }), @@ -2154,7 +2154,7 @@ describe("ContractClient — update/query operational errors", () => { opWorkflow: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({ ok: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { peek: { input: z.tuple([]), output: z.string() }, }, @@ -2324,7 +2324,7 @@ describe("ContractClient — raw escape hatch and accessors", () => { plain: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -2387,7 +2387,7 @@ describe("ContractClient — omittable input-less payloads (runtime)", () => { omittable: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({ ok: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { stop: defineSignal(), }, @@ -2503,7 +2503,7 @@ describe("ContractClient — search attribute VALUE validation (runtime)", () => kinds: defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", searchAttributes: { priority: defineSearchAttribute({ kind: "INT" }), placedAt: defineSearchAttribute({ kind: "DATETIME" }), @@ -2546,19 +2546,19 @@ describe("contract-declared idempotency", () => { const onceWorkflow = defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({ ok: z.boolean() }), - idempotency: "once-per-id", + startPolicy: "once-per-id", signals: { ping: { input: z.tuple([]) }, }, }); - // Simulates a definition that reaches the client without `idempotency` at + // Simulates a definition that reaches the client without `startPolicy` at // runtime despite the field now being required at the type level (e.g. a // contract assembled dynamically outside the type system, or an older // compiled artifact) — the `as unknown as typeof onceWorkflow` cast is the // point, not a mistake; it keeps every other generic (notably the `ping` // signal's literal name and tuple schema) intact so the calls below stay - // precisely typed. `client.ts`'s `definition.idempotency ? { + // precisely typed. `client.ts`'s `definition.startPolicy ? { // workflowIdReusePolicy: … } : {}` guard must stay defensive for exactly // this case. const plainWorkflow = { @@ -2709,7 +2709,7 @@ describe("contract-declared idempotency", () => { }); it("sends no policy when the contract declares none, on startWorkflow", async () => { - // `plainWorkflow` is missing `idempotency` at runtime (see its + // `plainWorkflow` is missing `startPolicy` at runtime (see its // definition above) — no `workflowIdReusePolicy` key at all should be // sent (not even `undefined`, which differs under // exactOptionalPropertyTypes). diff --git a/packages/client/src/client.ts b/packages/client/src/client.ts index 4255e76f..ea1f8c91 100644 --- a/packages/client/src/client.ts +++ b/packages/client/src/client.ts @@ -187,13 +187,26 @@ type WorkflowArgsField = ? { args?: ClientInferInput } : { args: ClientInferInput }; +export type WorkflowIdField = + TWorkflow["workflowId"] extends (input: never) => string + ? { + /** + * Derived from the payload by the contract — passing one here is a + * type error, because a caller-supplied ID is exactly what defeats a + * `startPolicy` of `"once-per-id"`. + */ + readonly workflowId?: never; + } + : { readonly workflowId: string }; + export type TypedWorkflowStartOptions< TContract extends ContractDefinition, TWorkflowName extends keyof TContract["workflows"] & string, > = Omit< WorkflowStartOptions, - "taskQueue" | "args" | "searchAttributes" | "typedSearchAttributes" + "taskQueue" | "args" | "searchAttributes" | "typedSearchAttributes" | "workflowId" > & + WorkflowIdField & WorkflowArgsField & { /** * Indexed search attributes for the started workflow. Keys and value types @@ -462,6 +475,15 @@ export type TypedWorkflowHandle = { type ResolvedWorkflow = { definition: TWorkflow; typedSearchAttributes: TypedSearchAttributes | undefined; + /** + * The input as the schema produced it. The caller's ORIGINAL value still + * crosses the wire (D1 — the worker parses on receive); this is here so a + * contract-declared `workflowId` derivation runs against the post-transform + * value, the way an activity's `idempotencyKey` does. Deriving from the raw + * payload would give `" ORD-1 "` and `"ORD-1"` two different workflow + * IDs, which is exactly the collision the derivation exists to force. + */ + validatedInput: unknown; }; /** @@ -499,7 +521,7 @@ function resolveDefinitionAndValidateInput< >( contract: TContract, workflowName: TWorkflowName, - workflowId: string, + workflowId: string | undefined, args: unknown, searchAttributes: Record | undefined, ): AsyncResult< @@ -525,10 +547,33 @@ function resolveDefinitionAndValidateInput< return Ok({ definition: definition as TContract["workflows"][TWorkflowName], typedSearchAttributes, + validatedInput: inputResult.value, }); }); } +/** + * The workflow ID a start should use: the contract's derivation applied to the + * validated input when the workflow declares one, and the caller's ID + * otherwise. + * + * A workflow that derives its ID also forbids `workflowId` in the start + * options at the type level, so the two can never disagree. + */ +function resolveWorkflowId( + definition: AnyWorkflowDefinition, + validatedInput: unknown, + callerWorkflowId: string | undefined, +): string { + // The structural slot types its parameter `never` so plain-object contracts + // stay assignable (see `WorkflowDefinition`); the value passed here is the + // validated input the derivation was written against. + const derive = definition.workflowId as ((input: unknown) => string) | undefined; + if (derive) return derive(validatedInput); + // Non-derived workflows type `workflowId` as required, so this is set. + return callerWorkflowId as string; +} + /** * Options for {@link TypedClient.create} — the single options-object shape * shared by the org's `Typed*.create()` factories. @@ -833,17 +878,18 @@ export class ContractClient { temporalOptions.workflowId, currentInput, searchAttributes as Record | undefined, - ).flatMap(({ definition, typedSearchAttributes }) => + ).flatMap(({ definition, typedSearchAttributes, validatedInput }) => // Transmit the caller's ORIGINAL args — the input was validated // above (fail early), but the worker parses on receive, so the // parsed value must not cross the wire (D1). An omitted payload // travels as empty args, not `[undefined]`. fromPromise( this.client.workflow.start(workflowName, { - ...(definition.idempotency - ? { workflowIdReusePolicy: _internal_reusePolicyFor(definition.idempotency) } + ...(definition.startPolicy + ? { workflowIdReusePolicy: _internal_reusePolicyFor(definition.startPolicy) } : {}), ...temporalOptions, + workflowId: resolveWorkflowId(definition, validatedInput, temporalOptions.workflowId), taskQueue: this.contract.taskQueue, args: currentInput === undefined ? [] : [currentInput], ...(typedSearchAttributes ? { typedSearchAttributes } : {}), @@ -980,13 +1026,14 @@ export class ContractClient { : Ok(resolved), ); }) - .flatMap(({ definition, typedSearchAttributes }) => + .flatMap(({ definition, typedSearchAttributes, validatedInput }) => fromPromise( this.client.workflow.signalWithStart(workflowName, { - ...(definition.idempotency - ? { workflowIdReusePolicy: _internal_reusePolicyFor(definition.idempotency) } + ...(definition.startPolicy + ? { workflowIdReusePolicy: _internal_reusePolicyFor(definition.startPolicy) } : {}), ...temporalOptions, + workflowId: resolveWorkflowId(definition, validatedInput, temporalOptions.workflowId), taskQueue: this.contract.taskQueue, args: currentInput === undefined ? [] : [currentInput], signal: signalName, @@ -1081,15 +1128,16 @@ export class ContractClient { temporalOptions.workflowId, currentInput, searchAttributes as Record | undefined, - ).flatMap(({ definition, typedSearchAttributes }) => + ).flatMap(({ definition, typedSearchAttributes, validatedInput }) => // Transmit the caller's ORIGINAL args (validated above, parsed by // the worker on receive — D1). fromPromise( this.client.workflow.execute(workflowName, { - ...(definition.idempotency - ? { workflowIdReusePolicy: _internal_reusePolicyFor(definition.idempotency) } + ...(definition.startPolicy + ? { workflowIdReusePolicy: _internal_reusePolicyFor(definition.startPolicy) } : {}), ...temporalOptions, + workflowId: resolveWorkflowId(definition, validatedInput, temporalOptions.workflowId), taskQueue: this.contract.taskQueue, args: currentInput === undefined ? [] : [currentInput], ...(typedSearchAttributes ? { typedSearchAttributes } : {}), diff --git a/packages/client/src/schedule.spec.ts b/packages/client/src/schedule.spec.ts index dc2505ff..cebe4e94 100644 --- a/packages/client/src/schedule.spec.ts +++ b/packages/client/src/schedule.spec.ts @@ -93,7 +93,7 @@ describe("TypedClient.schedule", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -168,7 +168,7 @@ describe("TypedClient.schedule", () => { transformer: defineWorkflow({ input: z.string().transform((s) => s.length), output: z.number(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -322,7 +322,7 @@ describe("TypedClient.schedule", () => { processOrder: defineWorkflow({ input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", searchAttributes: { customerId: defineSearchAttribute({ kind: "KEYWORD" }), priority: defineSearchAttribute({ kind: "INT" }), diff --git a/packages/client/src/types-inference.spec.ts b/packages/client/src/types-inference.spec.ts index a84907db..8b196b1c 100644 --- a/packages/client/src/types-inference.spec.ts +++ b/packages/client/src/types-inference.spec.ts @@ -56,7 +56,7 @@ const contractWithSignal = defineContract({ hasSignal: defineWorkflow({ input: z.object({ a: z.string() }), output: z.string(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { cancel: defineSignal({ input: z.object({ reason: z.string() }) }), }, @@ -70,7 +70,7 @@ const contractNoSignals = defineContract({ bare: defineWorkflow({ input: z.object({ a: z.string() }), output: z.string(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -81,7 +81,7 @@ const richContract = defineContract({ processOrder: defineWorkflow({ input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { // Payload-less signal — `defineSignal()` materializes an // UndefinedInputSchema, so the client-side payload is omittable. diff --git a/packages/client/src/workflow-id.spec.ts b/packages/client/src/workflow-id.spec.ts new file mode 100644 index 00000000..ba5d36af --- /dev/null +++ b/packages/client/src/workflow-id.spec.ts @@ -0,0 +1,149 @@ +/** + * Contract-derived workflow IDs. + * + * The point of the feature is that the ID and the start policy stop living in + * different places: `startPolicy: "once-per-id"` protects nothing if a caller + * is free to pass `crypto.randomUUID()`. These tests pin the ID that actually + * reaches Temporal, which is the only thing the policy sees. + */ +import { defineContract, defineWorkflow } from "@temporal-contract/contract"; +import type { Client } from "@temporalio/client"; +import { describe, expect, it, vi } from "vitest"; +import { z } from "zod"; + +import { TypedClient } from "./client.js"; + +const derivedContract = defineContract({ + taskQueue: "orders", + workflows: { + // Derives its ID: one execution per order, ever. + processOrder: defineWorkflow({ + input: z.object({ orderId: z.string(), amount: z.number() }), + output: z.object({ ok: z.boolean() }), + workflowId: ({ orderId }) => `order-${orderId}`, + startPolicy: "once-per-id", + }), + // Derives from a schema that trims, so the ID must come from the + // post-parse value. + processTrimmed: defineWorkflow({ + input: z.object({ orderId: z.string().transform((v) => v.trim()) }), + output: z.object({ ok: z.boolean() }), + workflowId: ({ orderId }) => `order-${orderId}`, + startPolicy: "once-per-id", + }), + // Declares no derivation: the caller still supplies the ID. + auditSweep: defineWorkflow({ + input: z.object({ day: z.string() }), + output: z.object({ ok: z.boolean() }), + startPolicy: "allow-duplicate", + }), + }, +}); + +function makeClient() { + const start = vi.fn().mockResolvedValue({ + workflowId: "assigned-by-temporal", + firstExecutionRunId: "run-1", + }); + const execute = vi.fn().mockResolvedValue({ ok: true }); + const raw = { + workflow: { start, execute, getHandle: vi.fn(), signalWithStart: vi.fn() }, + schedule: { create: vi.fn(), getHandle: vi.fn() }, + } as unknown as Client; + + return { raw, start, execute }; +} + +const bind = async (raw: Client) => + (await TypedClient.create({ client: raw }).get()).for(derivedContract); + +describe("contract-derived workflow IDs", () => { + it("derives the ID from the payload on startWorkflow", async () => { + const { raw, start } = makeClient(); + const orders = await bind(raw); + + await orders.startWorkflow("processOrder", { args: { orderId: "ORD-1", amount: 10 } }); + + expect(start).toHaveBeenCalledWith( + "processOrder", + expect.objectContaining({ workflowId: "order-ORD-1" }), + ); + }); + + it("derives the same ID for the same payload — which is what makes the policy bite", async () => { + const { raw, start } = makeClient(); + const orders = await bind(raw); + + await orders.startWorkflow("processOrder", { args: { orderId: "ORD-1", amount: 10 } }); + await orders.startWorkflow("processOrder", { args: { orderId: "ORD-1", amount: 10 } }); + + const [first, second] = start.mock.calls; + expect(first?.[1].workflowId).toBe(second?.[1].workflowId); + // And the policy that acts on it still travels with the start. + expect(first?.[1].workflowIdReusePolicy).toBe("REJECT_DUPLICATE"); + }); + + it("derives from the VALIDATED input, after schema transforms", async () => { + // Deriving from the raw payload would give " ORD-1 " and "ORD-1" two + // different IDs — two executions for one order, and the collision the + // derivation exists to force never happens. + const { raw, start } = makeClient(); + const orders = await bind(raw); + + await orders.startWorkflow("processTrimmed", { args: { orderId: " ORD-1 " } }); + + expect(start).toHaveBeenCalledWith( + "processTrimmed", + expect.objectContaining({ workflowId: "order-ORD-1" }), + ); + }); + + it("derives the ID on executeWorkflow too", async () => { + const { raw, execute } = makeClient(); + const orders = await bind(raw); + + await orders.executeWorkflow("processOrder", { args: { orderId: "ORD-9", amount: 1 } }); + + expect(execute).toHaveBeenCalledWith( + "processOrder", + expect.objectContaining({ workflowId: "order-ORD-9" }), + ); + }); + + it("still uses the caller's ID for a workflow that declares no derivation", async () => { + const { raw, start } = makeClient(); + const orders = await bind(raw); + + await orders.startWorkflow("auditSweep", { + workflowId: "sweep-2026-09-03", + args: { day: "2026-09-03" }, + }); + + expect(start).toHaveBeenCalledWith( + "auditSweep", + expect.objectContaining({ workflowId: "sweep-2026-09-03" }), + ); + }); +}); + +describe("contract-derived workflow IDs — types", () => { + it("rejects a caller-supplied ID for a derived workflow", async () => { + const { raw } = makeClient(); + const orders = await bind(raw); + + await orders.startWorkflow("processOrder", { + // @ts-expect-error -- the contract derives this workflow's ID; supplying + // one is what defeats `once-per-id`. + workflowId: crypto.randomUUID(), + args: { orderId: "ORD-1", amount: 10 }, + }); + }); + + it("still requires an ID for a workflow that declares no derivation", async () => { + const { raw } = makeClient(); + const orders = await bind(raw); + + // @ts-expect-error -- `workflowId` is required for a non-deriving workflow + await orders.startWorkflow("auditSweep", { args: { day: "2026-09-03" } }); + }); +}); diff --git a/packages/contract/src/builder.spec.ts b/packages/contract/src/builder.spec.ts index 647cbf43..fe00c247 100644 --- a/packages/contract/src/builder.spec.ts +++ b/packages/contract/src/builder.spec.ts @@ -19,7 +19,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -44,7 +44,7 @@ describe("Contract Builder", () => { simpleWorkflow: { input: z.object({ value: z.string() }), output: z.object({ result: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -74,7 +74,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { validateInventory: { input: z.object({ orderId: z.string() }), @@ -116,7 +116,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { cancel: { input: z.object({ reason: z.string() }), @@ -154,7 +154,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { getStatus: { input: z.object({}), @@ -196,7 +196,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", updates: { updateDiscount: { input: z.object({ percentage: z.number() }), @@ -238,17 +238,17 @@ describe("Contract Builder", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, cancelOrder: { input: z.object({ orderId: z.string(), reason: z.string() }), output: z.object({ cancelled: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, refundOrder: { input: z.object({ orderId: z.string(), amount: z.number() }), output: z.object({ refunded: z.boolean(), transactionId: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -271,7 +271,7 @@ describe("Contract Builder", () => { simpleWorkflow: { input: z.string(), // Single primitive output: z.object({ result: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, complexWorkflow: { input: z.object({ @@ -281,7 +281,7 @@ describe("Contract Builder", () => { amount: z.number(), }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, arrayWorkflow: { input: z.array( @@ -292,7 +292,7 @@ describe("Contract Builder", () => { }), ), output: z.object({ processed: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -315,7 +315,7 @@ describe("Contract Builder", () => { myWorkflow: { input: z.object({ id: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -337,7 +337,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), @@ -352,7 +352,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), @@ -392,7 +392,7 @@ describe("Contract Builder", () => { "invalid-name": { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), @@ -407,7 +407,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -428,7 +428,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { sendEmail: { input: z.object({}), @@ -457,7 +457,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { charge: { input: z.object({}), @@ -468,7 +468,7 @@ describe("Contract Builder", () => { processRefund: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { charge: { input: z.object({}), @@ -496,13 +496,13 @@ describe("Contract Builder", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { charge }, }, processRefund: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { charge }, }, }, @@ -523,7 +523,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { sendEmail }, }, }, @@ -540,7 +540,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -565,12 +565,12 @@ describe("Contract Builder", () => { aWorkflow: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, sendEmail: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -596,7 +596,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { processOrder: { input: z.object({}), @@ -619,7 +619,7 @@ describe("Contract Builder", () => { global: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { send: { input: z.object({}), @@ -630,7 +630,7 @@ describe("Contract Builder", () => { other: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { send: { input: z.object({}), @@ -653,7 +653,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { "cancel-order": { input: z.object({}), @@ -673,7 +673,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { "get-status": { input: z.object({}), @@ -694,7 +694,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", updates: { "update-amount": { input: z.object({}), @@ -715,7 +715,7 @@ describe("Contract Builder", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { sendEmail: { input: z.object({}), @@ -753,12 +753,12 @@ describe("Contract Builder", () => { process_order: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, $process: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -783,7 +783,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, // Deliberate typo of `activities` — TypeScript's generic inference @@ -804,7 +804,7 @@ describe("Contract Builder", () => { wf: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { shutdown: defineSignal() }, queries: { getStatus: defineQuery({ output: z.string() }) }, updates: { bump: defineUpdate({ output: z.number() }) }, @@ -823,7 +823,7 @@ describe("Contract Builder", () => { empty: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -846,7 +846,7 @@ describe("Contract Builder", () => { noInput: { input: z.void(), output: z.object({ result: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -869,7 +869,7 @@ describe("Contract Builder", () => { simple: { input: z.string(), output: z.string(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -890,7 +890,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -906,7 +906,7 @@ describe("Contract Builder", () => { // @ts-expect-error - Testing validation with missing input test: { output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), @@ -921,14 +921,14 @@ describe("Contract Builder", () => { // @ts-expect-error - Testing validation with missing output test: { input: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), ).toThrow("Contract validation failed"); }); - it("should throw when workflow idempotency is an invalid string", () => { + it("should throw when workflow startPolicy is an invalid string", () => { expect(() => defineContract({ taskQueue: "test", @@ -936,17 +936,17 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - // @ts-expect-error - Testing validation with an invalid idempotency literal - idempotency: "sometimes", + // @ts-expect-error - Testing validation with an invalid startPolicy literal + startPolicy: "sometimes", }, }, }), ).toThrow( - 'Contract validation failed: workflow "test": idempotency must be "once-per-id", "retry-if-failed", or "allow-duplicate"', + 'Contract validation failed: workflow "test": startPolicy must be "once-per-id", "retry-if-failed", or "allow-duplicate"', ); }); - it("should throw when workflow idempotency is not a string", () => { + it("should throw when workflow startPolicy is not a string", () => { expect(() => defineContract({ taskQueue: "test", @@ -954,8 +954,8 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - // @ts-expect-error - Testing validation with a non-string idempotency - idempotency: 42, + // @ts-expect-error - Testing validation with a non-string startPolicy + startPolicy: 42, }, }, }), @@ -970,7 +970,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -991,7 +991,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -1012,7 +1012,7 @@ describe("Contract Builder", () => { test: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { "invalid-name": { input: z.object({}), @@ -1034,7 +1034,7 @@ describe("Contract Builder", () => { // @ts-expect-error - Testing validation with invalid input type input: "not a schema", output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), @@ -1050,7 +1050,7 @@ describe("Contract Builder", () => { input: z.object({}), // @ts-expect-error - Testing validation with invalid output type output: { invalid: true }, - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), @@ -1068,7 +1068,7 @@ describe("Contract Builder", () => { processOrder: { input: v.object({ orderId: v.string() }), output: v.object({ status: v.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -1102,7 +1102,7 @@ describe("Contract Builder", () => { processOrder: { input: type({ orderId: "string" }), output: type({ status: "string" }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -1137,17 +1137,17 @@ describe("Contract Builder", () => { processZod: { input: z.object({ id: z.string() }), output: z.object({ result: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, processValibot: { input: v.object({ id: v.string() }), output: v.object({ result: v.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, processArkType: { input: type({ id: "string" }), output: type({ result: "string" }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -1178,7 +1178,7 @@ describe("Contract Builder — typed errors and default options", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", errors: { EmptyOrder: { data: z.object({ orderId: z.string() }) }, }, @@ -1214,7 +1214,7 @@ describe("Contract Builder — typed errors and default options", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { chargePayment: { input: z.object({}), @@ -1238,7 +1238,7 @@ describe("Contract Builder — typed errors and default options", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -1265,7 +1265,7 @@ describe("Contract Builder — typed errors and default options", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -1288,7 +1288,7 @@ describe("Contract Builder — typed errors and default options", () => { processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -1375,7 +1375,7 @@ describe("Contract Builder — Temporal-reserved names", () => { __temporal_cleanup: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), @@ -1391,7 +1391,7 @@ describe("Contract Builder — Temporal-reserved names", () => { wf: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { [reserved]: { input: z.object({}), output: z.object({}) }, }, @@ -1453,7 +1453,7 @@ describe("Contract Builder — Temporal-reserved names", () => { __internal_wf: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }), diff --git a/packages/contract/src/builder.ts b/packages/contract/src/builder.ts index d2205e69..012781ea 100644 --- a/packages/contract/src/builder.ts +++ b/packages/contract/src/builder.ts @@ -239,7 +239,7 @@ export function defineUpdate( * defineWorkflow({ * input: z.object({ orderId: z.string() }), * output: z.object({ status: z.string() }), - * idempotency: 'allow-duplicate', + * startPolicy: 'allow-duplicate', * searchAttributes: { * customerId: defineSearchAttribute({ kind: 'KEYWORD' }), * priority: defineSearchAttribute({ kind: 'INT' }), @@ -288,7 +288,7 @@ export function defineSearchAttribute( * // Payment already moved money on success — block a second successful * // run per order. A start is still retryable after a genuinely failed * // attempt (e.g. a declined payment, where no charge went through). - * idempotency: 'retry-if-failed', + * startPolicy: 'retry-if-failed', * activities: { * chargePayment: defineActivity({ * input: z.object({ orderId: z.string(), amount: z.number() }), @@ -303,8 +303,19 @@ export function defineSearchAttribute( * }); * ``` */ -export function defineWorkflow( - definition: TWorkflow, +export function defineWorkflow< + TInput extends AnySchema, + TWorkflow extends AnyWorkflowDefinition & { readonly input: TInput }, +>( + definition: TWorkflow & { + readonly input: TInput; + /** + * Re-stated against the bound input schema (the structural definition + * types it `never` — see {@link WorkflowDefinition}), so this lambda's + * parameter is contextually typed as the workflow's validated input. + */ + readonly workflowId?: (input: StandardSchemaV1.InferOutput) => string; + }, ): TWorkflow { return definition; } @@ -365,7 +376,7 @@ export function defineWorkflow( * // Payment already moved money on success — block a second successful * // run per order. A start is still retryable after a genuinely failed * // attempt (e.g. a declined payment, where no charge went through). - * idempotency: 'retry-if-failed', + * startPolicy: 'retry-if-failed', * activities: { chargePayment }, * }); * @@ -746,24 +757,24 @@ function validateWorkflowDefinition(context: string, definition: unknown): void } assertSchema(context, "input", definition["input"]); assertSchema(context, "output", definition["output"]); - const idempotency = definition["idempotency"]; - // `idempotency` is required at the *type* level (`WorkflowDefinition`, + const startPolicy = definition["startPolicy"]; + // `startPolicy` is required at the *type* level (`WorkflowDefinition`, // types.ts), but this runtime check deliberately still accepts `undefined` // here — tightening it to reject a missing field would be "finishing the // flip" for real, and it's load-bearing: it's what lets a definition reach - // the client/worker without `idempotency` at runtime despite the type + // the client/worker without `startPolicy` at runtime despite the type // requiring it (e.g. a contract assembled outside the type system, or an // older compiled artifact) without failing contract validation. The - // client's/worker's own `definition.idempotency ? {...} : {}` guards stay + // client's/worker's own `definition.startPolicy ? {...} : {}` guards stay // defensive for exactly that case, and the `plainWorkflow` fixture in // client.spec.ts exists to prove it. if ( - idempotency !== undefined && - idempotency !== "once-per-id" && - idempotency !== "retry-if-failed" && - idempotency !== "allow-duplicate" + startPolicy !== undefined && + startPolicy !== "once-per-id" && + startPolicy !== "retry-if-failed" && + startPolicy !== "allow-duplicate" ) { - fail(`${context}: idempotency must be "once-per-id", "retry-if-failed", or "allow-duplicate"`); + fail(`${context}: startPolicy must be "once-per-id", "retry-if-failed", or "allow-duplicate"`); } validateDefinitionMap( context, diff --git a/packages/contract/src/helpers.spec.ts b/packages/contract/src/helpers.spec.ts index ea61f38e..af2117e1 100644 --- a/packages/contract/src/helpers.spec.ts +++ b/packages/contract/src/helpers.spec.ts @@ -148,7 +148,7 @@ describe("Helper Functions", () => { const workflow = defineWorkflow({ input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); expect(workflow).toEqual( @@ -163,7 +163,7 @@ describe("Helper Functions", () => { const workflow = defineWorkflow({ input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { processPayment: { input: z.object({ amount: z.number() }), diff --git a/packages/contract/src/idempotency.ts b/packages/contract/src/idempotency.ts index a57fc1ea..ffea5a1d 100644 --- a/packages/contract/src/idempotency.ts +++ b/packages/contract/src/idempotency.ts @@ -11,7 +11,7 @@ * a per-call option because different callers legitimately want different * answers to "one is already in flight". */ -export type IdempotencyMode = +export type WorkflowStartPolicy = /** This workflow ID may run exactly once, ever. */ | "once-per-id" /** Re-runnable only if the previous attempt did not succeed. */ @@ -37,13 +37,23 @@ export type WorkflowIdReusePolicy = * cannot drift; `Record` makes a newly added mode a * compile error until it is mapped. */ -const REUSE_POLICY: Record = { +const REUSE_POLICY: Record = { "once-per-id": "REJECT_DUPLICATE", "retry-if-failed": "ALLOW_DUPLICATE_FAILED_ONLY", "allow-duplicate": "ALLOW_DUPLICATE", }; /** Translate a contract's declared idempotency mode to Temporal's policy. */ -export function reusePolicyFor(mode: IdempotencyMode): WorkflowIdReusePolicy { +export function reusePolicyFor(mode: WorkflowStartPolicy): WorkflowIdReusePolicy { return REUSE_POLICY[mode]; } + +/** + * @deprecated Renamed to {@link WorkflowStartPolicy}, and the field that + * carries it from `startPolicy` to `startPolicy`: it governs + * `workflowIdReusePolicy` — whether a workflow ID may be reused after a + * Closed run — and never made a workflow idempotent. For an activity running + * twice under Temporal's at-least-once guarantee, see an activity's + * `idempotencyKey`. + */ +export type IdempotencyMode = WorkflowStartPolicy; diff --git a/packages/contract/src/internal.ts b/packages/contract/src/internal.ts index 9f988a36..f38643ef 100644 --- a/packages/contract/src/internal.ts +++ b/packages/contract/src/internal.ts @@ -22,9 +22,9 @@ export { } from "./errors-impl.js"; /** - * Mode→policy mapping for `idempotency` — re-exported under the + * Mode→policy mapping for `startPolicy` — re-exported under the * `_internal_` prefix used throughout this subpath. Not part of the public - * API: contract authors only ever set `idempotency` on `defineWorkflow`; the + * API: contract authors only ever set `startPolicy` on `defineWorkflow`; the * client and worker are the ones that translate it to Temporal's * `workflowIdReusePolicy` via this function, so it lives here rather than on * `.` alongside the public `IdempotencyMode` type. diff --git a/packages/contract/src/types-inference.spec.ts b/packages/contract/src/types-inference.spec.ts index 64de8b0b..99bff92f 100644 --- a/packages/contract/src/types-inference.spec.ts +++ b/packages/contract/src/types-inference.spec.ts @@ -35,12 +35,12 @@ const contract = defineContract({ processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, sendNotification: { input: z.object({ userId: z.string() }), output: z.void(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -73,7 +73,7 @@ describe("contract inference utilities", () => { wf: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }); @@ -122,7 +122,7 @@ describe("WorkflowDefinition generic preservation (audit fix #2)", () => { const wf = defineWorkflow({ input: z.object({ a: z.string() }), output: z.string(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); // Before the fix, these were widened to `AnySchema`/`StandardSchemaV1` @@ -138,7 +138,7 @@ describe("WorkflowDefinition generic preservation (audit fix #2)", () => { p: defineWorkflow({ input: z.object({ a: z.string() }), output: z.string(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -155,7 +155,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { hasSignal: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { cancel: defineSignal({ input: z.object({ reason: z.string() }) }), }, @@ -169,7 +169,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { bare: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -198,7 +198,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { const wfA = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { cancel: defineSignal({ input: z.object({ reason: z.string() }) }), }, @@ -206,7 +206,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { const wfB = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { pause: defineSignal({ input: z.object({}) }), }, @@ -224,7 +224,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { const wfA = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { getStatus: defineQuery({ input: z.object({}), output: z.string() }), }, @@ -235,7 +235,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { const wfB = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { getCount: defineQuery({ input: z.object({}), output: z.number() }), }, @@ -254,7 +254,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { const wf = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { getStatus: defineQuery({ input: z.object({}), output: z.string() }), }, @@ -266,7 +266,7 @@ describe("Signal/query/update name helpers (audit fix #3)", () => { const wf = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", updates: { bump: defineUpdate({ input: z.object({}), output: z.number() }), }, @@ -304,7 +304,7 @@ describe("input-less signal/query/update definitions", () => { const wf = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { shutdown: defineSignal() }, queries: { getStatus: defineQuery({ output: z.string() }) }, updates: { bump: defineUpdate({ output: z.number() }) }, diff --git a/packages/contract/src/types.spec.ts b/packages/contract/src/types.spec.ts index 00b07140..3b6aa1ce 100644 --- a/packages/contract/src/types.spec.ts +++ b/packages/contract/src/types.spec.ts @@ -80,7 +80,7 @@ describe("Core Types", () => { const workflowDef: AnyWorkflowDefinition = { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { processPayment: { input: z.object({ amount: z.number() }), @@ -107,7 +107,7 @@ describe("Core Types", () => { const workflowDef: AnyWorkflowDefinition = { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { cancel: { input: z.object({ reason: z.string() }), @@ -128,7 +128,7 @@ describe("Core Types", () => { const workflowDef: AnyWorkflowDefinition = { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { getStatus: { input: z.object({ detailed: z.boolean() }), @@ -153,7 +153,7 @@ describe("Core Types", () => { const workflowDef: AnyWorkflowDefinition = { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", updates: { changeQuantity: { input: z.object({ quantity: z.number() }), @@ -183,7 +183,7 @@ describe("Core Types", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, }; @@ -208,7 +208,7 @@ describe("Core Types", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -237,12 +237,12 @@ describe("Core Types", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, sendNotification: { input: z.object({ userId: z.string() }), output: z.void(), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, } satisfies ContractDefinition; @@ -286,7 +286,7 @@ describe("Core Types", () => { processOrder: { input: z.object({ orderId: z.string() }), output: z.object({ success: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, } satisfies ContractDefinition; diff --git a/packages/contract/src/types.ts b/packages/contract/src/types.ts index 3f1b437c..60729462 100644 --- a/packages/contract/src/types.ts +++ b/packages/contract/src/types.ts @@ -1,6 +1,6 @@ import type { StandardSchemaV1 } from "@standard-schema/spec"; -import type { IdempotencyMode } from "./idempotency.js"; +import type { WorkflowStartPolicy } from "./idempotency.js"; /** * Base types for validation schemas @@ -273,6 +273,40 @@ export type WorkflowDefinition< > = { readonly input: TInput; readonly output: TOutput; + /** + * Derive this workflow's **workflow ID** from its input. + * + * Declaring it moves the ID from the caller to the contract: every + * `startWorkflow` / `executeWorkflow` / `signalWithStart` computes the ID + * from the payload, and passing one explicitly becomes a type error. That + * is what makes {@link startPolicy} mean anything — a caller free to pass + * `crypto.randomUUID()` defeats `"once-per-id"` silently, because every + * start gets a fresh ID and the policy never fires. + * + * The function receives the **validated** input and must be pure: the same + * payload has to produce the same ID on every call, or two starts of the + * same logical request will not collide. + * + * NOT applied to `schedule.create`, which generates one ID per firing — + * a scheduled run wants a distinct execution, not deduplication. + * + * The parameter is typed `never` here for the same reason as an activity's + * `idempotencyKey` (a property-position function type is contravariant, and + * plain-object contracts must stay assignable); `defineWorkflow` re-states + * the slot against the bound input schema, so the lambda written there is + * contextually typed. + * + * @example + * ```ts + * const processOrder = defineWorkflow({ + * input: OrderSchema, + * output: OrderResultSchema, + * workflowId: ({ orderId }) => orderId, + * startPolicy: "retry-if-failed", + * }); + * ``` + */ + readonly workflowId?: (input: never) => string; /** * Whether this workflow is safe to re-run under a workflow ID that has * already been used. Applied by the client to every `startWorkflow` / @@ -287,8 +321,14 @@ export type WorkflowDefinition< * * Required so the question is asked once per workflow rather than * silently inheriting Temporal's `ALLOW_DUPLICATE`. + * + * Named for what it governs — Temporal's `workflowIdReusePolicy` — rather + * than for idempotency in general. It does **not** make a workflow + * idempotent, and it says nothing about an activity running twice under + * Temporal's at-least-once guarantee; that is an activity's + * `idempotencyKey`. */ - readonly idempotency: IdempotencyMode; + readonly startPolicy: WorkflowStartPolicy; readonly activities?: TActivities; readonly signals?: TSignals; readonly queries?: TQueries; diff --git a/packages/testing/src/__tests__/test.contract.ts b/packages/testing/src/__tests__/test.contract.ts index 257d82fe..1217c495 100644 --- a/packages/testing/src/__tests__/test.contract.ts +++ b/packages/testing/src/__tests__/test.contract.ts @@ -13,7 +13,7 @@ const decorate = defineActivity({ const greet = defineWorkflow({ input: z.object({ name: z.string() }), output: z.object({ message: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { decorate }, }); diff --git a/packages/testing/src/workflow-bundle.spec.ts b/packages/testing/src/workflow-bundle.spec.ts index 6b95c2df..d07245f8 100644 --- a/packages/testing/src/workflow-bundle.spec.ts +++ b/packages/testing/src/workflow-bundle.spec.ts @@ -12,7 +12,7 @@ const contract = defineContract({ noop: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); diff --git a/packages/worker/src/__tests__/activity-options.contract.ts b/packages/worker/src/__tests__/activity-options.contract.ts index 2238e093..fcb3e260 100644 --- a/packages/worker/src/__tests__/activity-options.contract.ts +++ b/packages/worker/src/__tests__/activity-options.contract.ts @@ -26,7 +26,7 @@ const slowActivity = defineActivity({ const runsActivity = defineWorkflow({ input: sleepInput, output: z.object({ outcome: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { slowActivity }, }); @@ -128,7 +128,7 @@ const resolvesLayeredOptions = defineWorkflow({ flaky: z.string(), globalTimeout: z.string(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { contractTimeoutActivity, usesDefaultActivity, flakyActivity }, }); diff --git a/packages/worker/src/__tests__/cancellation.contract.ts b/packages/worker/src/__tests__/cancellation.contract.ts index ca92311f..765cf42f 100644 --- a/packages/worker/src/__tests__/cancellation.contract.ts +++ b/packages/worker/src/__tests__/cancellation.contract.ts @@ -58,7 +58,7 @@ const slowActivity = defineActivity({ const swallowsCancellation = defineWorkflow({ input: z.object({}), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); /** @@ -70,7 +70,7 @@ const swallowsCancellation = defineWorkflow({ const honorsCancellation = defineWorkflow({ input: z.object({}), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); /** @@ -81,7 +81,7 @@ const honorsCancellation = defineWorkflow({ const nonCancellableWorkflow = defineWorkflow({ input: z.object({}), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); /** @@ -103,7 +103,7 @@ const scopeMode = z.enum([ const scopeMechanics = defineWorkflow({ input: z.object({ mode: scopeMode }), output: z.object({ outcome: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); export const cancellationContract = defineContract({ diff --git a/packages/worker/src/__tests__/child-idempotency.contract.ts b/packages/worker/src/__tests__/child-idempotency.contract.ts index b2d77649..7bf427a5 100644 --- a/packages/worker/src/__tests__/child-idempotency.contract.ts +++ b/packages/worker/src/__tests__/child-idempotency.contract.ts @@ -20,7 +20,7 @@ const childOutput = z.object({ ok: z.boolean() }); const onceChild = defineWorkflow({ input: childInput, output: childOutput, - idempotency: "once-per-id", + startPolicy: "once-per-id", }); /** @@ -53,7 +53,7 @@ const parent = defineWorkflow({ // workflow ID (see the spec file), so `parent` itself is never re-run // under a reused ID — its own idempotency mode is not under test here. // `"allow-duplicate"` is Temporal's own default, chosen so it stays inert. - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); export const childIdempotencyContract = defineContract({ diff --git a/packages/worker/src/__tests__/child-idempotency.inprocess.spec.ts b/packages/worker/src/__tests__/child-idempotency.inprocess.spec.ts index 72603d08..0922db1c 100644 --- a/packages/worker/src/__tests__/child-idempotency.inprocess.spec.ts +++ b/packages/worker/src/__tests__/child-idempotency.inprocess.spec.ts @@ -13,7 +13,7 @@ import { childIdempotencyContract } from "./child-idempotency.contract.js"; * Real-server (time-skipping) coverage of contract-declared idempotency at * the CHILD-WORKFLOW boundary — `context.startChildWorkflow` and * `context.executeChildWorkflow`, both of which now apply the contract's - * declared `idempotency` mode as `workflowIdReusePolicy` + * declared `startPolicy` mode as `workflowIdReusePolicy` * (`child-workflow.ts`). * * This replaces an earlier version of this coverage that lived in diff --git a/packages/worker/src/__tests__/child-wire.contract.ts b/packages/worker/src/__tests__/child-wire.contract.ts index c62694bf..e36844a8 100644 --- a/packages/worker/src/__tests__/child-wire.contract.ts +++ b/packages/worker/src/__tests__/child-wire.contract.ts @@ -43,14 +43,14 @@ export const childWireContract = defineContract({ entryTransform: defineWorkflow({ input: z.object({ text: z.string().transform((s) => `${s}!`) }), output: z.object({ text: z.string(), n: z.number().transform((n) => n * 2) }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** Deliberately returns a value the output schema rejects. */ entryInvalidOutput: defineWorkflow({ input: z.object({}), output: z.object({ n: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** @@ -62,7 +62,7 @@ export const childWireContract = defineContract({ child: defineWorkflow({ input: z.object({ label: z.string().transform((s) => `${s}!`) }), output: z.object({ label: z.string(), n: z.number().transform((n) => n * 2) }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** @@ -81,7 +81,7 @@ export const childWireContract = defineContract({ firstExecutionRunId: z.string().optional(), childWorkflowId: z.string().optional(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** @@ -93,7 +93,7 @@ export const childWireContract = defineContract({ signalful: defineWorkflow({ input: z.object({}), output: z.object({ noteText: z.string().nullable() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { note: defineSignal({ input: z.object({ text: z.string().transform((s) => `${s}!`) }) }), finish: defineSignal(), @@ -118,7 +118,7 @@ export const childWireContract = defineContract({ noteText: z.string().nullable(), childWorkflowId: z.string().optional(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); diff --git a/packages/worker/src/__tests__/continue-as-new.contract.ts b/packages/worker/src/__tests__/continue-as-new.contract.ts index c0d049c7..16e09f4a 100644 --- a/packages/worker/src/__tests__/continue-as-new.contract.ts +++ b/packages/worker/src/__tests__/continue-as-new.contract.ts @@ -21,7 +21,7 @@ export const continueAsNewContract = defineContract({ accumulate: defineWorkflow({ input: z.object({ cursor: z.number(), total: z.number(), smuggle: z.boolean().optional() }), output: z.object({ total: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** @@ -32,7 +32,7 @@ export const continueAsNewContract = defineContract({ invalidContinuation: defineWorkflow({ input: z.object({ n: z.number() }), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** @@ -43,7 +43,7 @@ export const continueAsNewContract = defineContract({ transformOnce: defineWorkflow({ input: z.object({ text: z.string().transform((s) => `${s}!`), hops: z.number() }), output: z.object({ text: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** @@ -59,7 +59,7 @@ export const continueAsNewContract = defineContract({ otherTaskQueue: z.string(), }), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), /** @@ -75,7 +75,7 @@ export const continueAsNewContract = defineContract({ workflows: z.union([z.record(z.string(), z.unknown()), z.null()]), }), output: z.object({ status: z.string(), hop: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); @@ -87,7 +87,7 @@ export const otherContract = defineContract({ archive: defineWorkflow({ input: z.object({ batchId: z.string() }), output: z.object({ batchId: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); diff --git a/packages/worker/src/__tests__/handlers.contract.ts b/packages/worker/src/__tests__/handlers.contract.ts index 8de88456..52c1dedb 100644 --- a/packages/worker/src/__tests__/handlers.contract.ts +++ b/packages/worker/src/__tests__/handlers.contract.ts @@ -98,7 +98,7 @@ const asyncOutputUpdate = defineUpdate({ const counter = defineWorkflow({ input: z.object({}), output: z.object({ total: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { bump, finish }, queries: { peek, describe, brokenOutput }, updates: { applyDelta, brokenOutputUpdate, asyncOutputUpdate }, @@ -112,7 +112,7 @@ const asyncCheckedQuery = defineQuery({ const bindsAsyncQuerySchema = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { asyncCheckedQuery }, }); @@ -128,7 +128,7 @@ const asyncCheckedQueryOutput = defineQuery({ output: alwaysAsyncSchema }); const bindsAsyncQueryOutputSchema = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { asyncCheckedQueryOutput }, }); @@ -146,7 +146,7 @@ const asyncCheckedUpdateInput = defineUpdate({ const bindsAsyncUpdateSchema = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", updates: { asyncCheckedUpdateInput }, }); @@ -234,7 +234,7 @@ const thenableDodging = defineQuery({ const probeEdgeCases = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", queries: { syncThrowProbe, probeDodging, thenableDodging }, }); @@ -257,7 +257,7 @@ const poke = defineUpdate({ input: transformingText, output: transformingOutput const transformWorkflow = defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { note }, queries: { peekNote, peekText }, updates: { poke }, diff --git a/packages/worker/src/__tests__/idempotency.contract.ts b/packages/worker/src/__tests__/idempotency.contract.ts index 8523fff2..5696c7b3 100644 --- a/packages/worker/src/__tests__/idempotency.contract.ts +++ b/packages/worker/src/__tests__/idempotency.contract.ts @@ -13,19 +13,19 @@ const output = z.object({ ok: z.boolean() }); const onceWorkflow = defineWorkflow({ input, output, - idempotency: "once-per-id", + startPolicy: "once-per-id", }); const retryWorkflow = defineWorkflow({ input, output, - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", }); const allowWorkflow = defineWorkflow({ input, output, - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); export const idempotencyContract = defineContract({ diff --git a/packages/worker/src/__tests__/inprocess.contract.ts b/packages/worker/src/__tests__/inprocess.contract.ts index 9ed14e34..31a4b8c1 100644 --- a/packages/worker/src/__tests__/inprocess.contract.ts +++ b/packages/worker/src/__tests__/inprocess.contract.ts @@ -20,7 +20,7 @@ const charge = defineActivity({ const placeOrder = defineWorkflow({ input: z.object({ orderId: z.string(), amount: z.number() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", errors: { EmptyOrder: { data: z.object({ orderId: z.string() }), @@ -39,7 +39,7 @@ const placeOrder = defineWorkflow({ const waitForever = defineWorkflow({ input: z.object({}), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); export const inprocessContract = defineContract({ diff --git a/packages/worker/src/__tests__/propagation.contract.ts b/packages/worker/src/__tests__/propagation.contract.ts index e3ad0d5a..296a29bd 100644 --- a/packages/worker/src/__tests__/propagation.contract.ts +++ b/packages/worker/src/__tests__/propagation.contract.ts @@ -39,7 +39,7 @@ const alwaysFailsWithErrors = defineActivity({ const propagatesFailure = defineWorkflow({ input: z.object({}), output: z.object({ reached: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { alwaysFailsNoErrors }, }); @@ -47,7 +47,7 @@ const propagatesFailure = defineWorkflow({ const handlesFailure = defineWorkflow({ input: z.object({}), output: z.object({ outcome: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { alwaysFailsNoErrors }, }); diff --git a/packages/worker/src/__tests__/registration.contract.ts b/packages/worker/src/__tests__/registration.contract.ts index f13f382b..8bfa4b48 100644 --- a/packages/worker/src/__tests__/registration.contract.ts +++ b/packages/worker/src/__tests__/registration.contract.ts @@ -10,13 +10,13 @@ import { z } from "zod"; const alpha = defineWorkflow({ input: z.object({ value: z.string() }), output: z.object({ result: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); const beta = defineWorkflow({ input: z.object({ n: z.number() }), output: z.object({ doubled: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); export const registrationContract = defineContract({ diff --git a/packages/worker/src/__tests__/rehydration.contract.ts b/packages/worker/src/__tests__/rehydration.contract.ts index 62fd31df..ed755508 100644 --- a/packages/worker/src/__tests__/rehydration.contract.ts +++ b/packages/worker/src/__tests__/rehydration.contract.ts @@ -25,7 +25,7 @@ const charge = defineActivity({ const quote = defineWorkflow({ input: z.object({ mode: z.string() }), output: z.object({ classification: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", errors: { QuoteExpired: { data: z.object({ quoteId: z.string() }), @@ -49,7 +49,7 @@ export const rehydrationWorkerContract = defineContract({ const quoteSkewed = defineWorkflow({ input: z.object({ mode: z.string() }), output: z.object({ classification: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", errors: { QuoteExpired: { data: z.object({ quoteId: z.string().startsWith("Q-") }), diff --git a/packages/worker/src/__tests__/retry.contract.ts b/packages/worker/src/__tests__/retry.contract.ts index 92138ba5..67d69307 100644 --- a/packages/worker/src/__tests__/retry.contract.ts +++ b/packages/worker/src/__tests__/retry.contract.ts @@ -29,7 +29,7 @@ const flaky = defineActivity({ const runsFlaky = defineWorkflow({ input: z.object({ mode: z.enum(["terminal", "retryable"]) }), output: z.object({ outcome: z.string(), attempts: z.number() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { flaky }, }); diff --git a/packages/worker/src/__tests__/routing.contract.ts b/packages/worker/src/__tests__/routing.contract.ts index 629795f9..ec88aec1 100644 --- a/packages/worker/src/__tests__/routing.contract.ts +++ b/packages/worker/src/__tests__/routing.contract.ts @@ -16,7 +16,7 @@ const reportQueue = defineActivity({ const routedFlow = defineWorkflow({ input: z.object({}), output: z.object({ handledBy: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { reportQueue }, }); diff --git a/packages/worker/src/__tests__/saga.contract.ts b/packages/worker/src/__tests__/saga.contract.ts index 96f944c4..ff7be29b 100644 --- a/packages/worker/src/__tests__/saga.contract.ts +++ b/packages/worker/src/__tests__/saga.contract.ts @@ -48,7 +48,7 @@ const refund = defineActivity({ const fulfil = defineWorkflow({ input: z.object({ mode: z.enum(["declared", "unmodelled"]) }), output: z.object({ failedWith: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { ship, refund }, }); @@ -60,7 +60,7 @@ const fulfil = defineWorkflow({ const fulfilUntilCancelled = defineWorkflow({ input: z.object({}), output: z.object({ failedWith: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: {}, }); diff --git a/packages/worker/src/__tests__/test.contract.ts b/packages/worker/src/__tests__/test.contract.ts index 4a803ff2..7c181a81 100644 --- a/packages/worker/src/__tests__/test.contract.ts +++ b/packages/worker/src/__tests__/test.contract.ts @@ -23,7 +23,7 @@ export const testContract = defineContract({ output: z.object({ result: z.string(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), // Workflow with its own activities @@ -38,7 +38,7 @@ export const testContract = defineContract({ transactionId: z.string().optional(), reason: z.string().optional(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { processPayment: defineActivity({ input: z.object({ @@ -73,7 +73,7 @@ export const testContract = defineContract({ output: z.object({ finalValue: z.number(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", signals: { increment: defineSignal({ input: z.object({ @@ -109,7 +109,7 @@ export const testContract = defineContract({ output: z.object({ results: z.array(z.string()), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), // Child workflow to be called from parent @@ -120,7 +120,7 @@ export const testContract = defineContract({ output: z.object({ message: z.string(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), // Workflow that calls a failable activity for error handling tests @@ -131,7 +131,7 @@ export const testContract = defineContract({ output: z.object({ success: z.boolean(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, activities: { diff --git a/packages/worker/src/__tests__/timeouts.contract.ts b/packages/worker/src/__tests__/timeouts.contract.ts index c9b5d1ac..004c1fc8 100644 --- a/packages/worker/src/__tests__/timeouts.contract.ts +++ b/packages/worker/src/__tests__/timeouts.contract.ts @@ -30,7 +30,7 @@ const reportsLayered = defineWorkflow({ scheduleToCloseMs: z.number(), heartbeatMs: z.number(), }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { reportsTimeouts }, }); diff --git a/packages/worker/src/activity-contract-errors.spec.ts b/packages/worker/src/activity-contract-errors.spec.ts index 41688fc2..fd43228d 100644 --- a/packages/worker/src/activity-contract-errors.spec.ts +++ b/packages/worker/src/activity-contract-errors.spec.ts @@ -27,7 +27,7 @@ const contract = defineContract({ processOrder: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { chargePayment: { input: z.object({ amount: z.number() }), @@ -103,7 +103,7 @@ describe("declareActivitiesHandler — contract errors", () => { const transformingContract = defineContract({ taskQueue: "test-queue", workflows: { - noop: { input: z.object({}), output: z.object({}), idempotency: "allow-duplicate" }, + noop: { input: z.object({}), output: z.object({}), startPolicy: "allow-duplicate" }, }, activities: { flaky: { diff --git a/packages/worker/src/activity-idempotency.spec.ts b/packages/worker/src/activity-idempotency.spec.ts index 2da29e96..6f22ddc8 100644 --- a/packages/worker/src/activity-idempotency.spec.ts +++ b/packages/worker/src/activity-idempotency.spec.ts @@ -24,7 +24,7 @@ const contract = { checkout: { input: z.object({ orderId: z.string() }), output: z.object({ done: z.boolean() }), - idempotency: "retry-if-failed", + startPolicy: "retry-if-failed", }, }, activities: { diff --git a/packages/worker/src/activity.spec.ts b/packages/worker/src/activity.spec.ts index 06c7a6a6..e17b921a 100644 --- a/packages/worker/src/activity.spec.ts +++ b/packages/worker/src/activity.spec.ts @@ -20,7 +20,7 @@ describe("Worker unthrown Package", () => { testWorkflow: { input: z.object({ value: z.string() }), output: z.object({ result: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { @@ -266,7 +266,7 @@ describe("Worker unthrown Package", () => { orderWorkflow: { input: z.object({ orderId: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { validateOrder: { input: z.object({ orderId: z.string() }), @@ -329,7 +329,7 @@ describe("Worker unthrown Package", () => { noopWorkflow: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, } satisfies ContractDefinition; @@ -381,7 +381,7 @@ describe("Worker unthrown Package", () => { orderWorkflow: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { validateOrder: { input: z.object({}), @@ -438,13 +438,13 @@ describe("Worker unthrown Package", () => { alpha: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { sharedActivity: sharedDef }, }, beta: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { sharedActivity: sharedDef }, }, }, @@ -507,7 +507,7 @@ describe("Worker unthrown Package", () => { alpha: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { sharedActivity: sharedDef }, }, }, @@ -537,7 +537,7 @@ describe("Worker unthrown Package", () => { conflicted: { input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }, }, activities: { diff --git a/packages/worker/src/child-workflow.ts b/packages/worker/src/child-workflow.ts index cb004413..0e8dc056 100644 --- a/packages/worker/src/child-workflow.ts +++ b/packages/worker/src/child-workflow.ts @@ -37,7 +37,7 @@ import type { ClientInferInput, ClientInferOutput, SignalDefOf } from "./types.j /** * Options for starting a child workflow. `taskQueue` and `args` come from * the contract, which also supplies a default `workflowIdReusePolicy` - * derived from the target workflow's declared `idempotency` mode; everything + * derived from the target workflow's declared `startPolicy` mode; everything * else — including an explicit `workflowIdReusePolicy` here, which overrides * that default — is forwarded to Temporal's `startChild` / `executeChild`. * @@ -297,8 +297,8 @@ export function createStartChildWorkflow< // the child workflow on receive (D1). const { args: childArgs, ...temporalOptions } = options; const handle = await startChild(childWorkflowName, { - ...(childDefinition.idempotency - ? { workflowIdReusePolicy: _internal_reusePolicyFor(childDefinition.idempotency) } + ...(childDefinition.startPolicy + ? { workflowIdReusePolicy: _internal_reusePolicyFor(childDefinition.startPolicy) } : {}), ...temporalOptions, taskQueue, @@ -349,8 +349,8 @@ export function createExecuteChildWorkflow< // the child workflow on receive (D1). const { args: childArgs, ...temporalOptions } = options; const result = await executeChild(childWorkflowName, { - ...(childDefinition.idempotency - ? { workflowIdReusePolicy: _internal_reusePolicyFor(childDefinition.idempotency) } + ...(childDefinition.startPolicy + ? { workflowIdReusePolicy: _internal_reusePolicyFor(childDefinition.startPolicy) } : {}), ...temporalOptions, taskQueue, diff --git a/packages/worker/src/handlers.spec.ts b/packages/worker/src/handlers.spec.ts index 3539596a..40b52abb 100644 --- a/packages/worker/src/handlers.spec.ts +++ b/packages/worker/src/handlers.spec.ts @@ -37,7 +37,7 @@ const baseDefinition = (): AnyWorkflowDefinition => ({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }) as unknown as AnyWorkflowDefinition; const withSignals = (signals: Record): AnyWorkflowDefinition => diff --git a/packages/worker/src/types-inference.spec.ts b/packages/worker/src/types-inference.spec.ts index 66d515b3..5e600f59 100644 --- a/packages/worker/src/types-inference.spec.ts +++ b/packages/worker/src/types-inference.spec.ts @@ -98,7 +98,7 @@ const logDef = defineActivity({ const orderWorkflowDef = defineWorkflow({ input: z.object({ id: z.string() }), output: z.object({ status: z.string() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { validateOrder: validateOrderDef }, signals: { cancel: defineSignal({ input: z.object({ reason: z.string() }) }), @@ -108,7 +108,7 @@ const orderWorkflowDef = defineWorkflow({ const otherWorkflowDef = defineWorkflow({ input: z.object({ batchId: z.string() }), output: z.object({ archived: z.boolean() }), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }); const inferenceContract = defineContract({ diff --git a/packages/worker/src/workflow-options.spec.ts b/packages/worker/src/workflow-options.spec.ts index b001e315..4311def0 100644 --- a/packages/worker/src/workflow-options.spec.ts +++ b/packages/worker/src/workflow-options.spec.ts @@ -19,7 +19,7 @@ const contract = defineContract({ processOrder: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", activities: { chargePayment: defineActivity({ input: z.object({ amount: z.number() }), @@ -34,7 +34,7 @@ const contract = defineContract({ other: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, activities: { diff --git a/packages/worker/src/workflow.spec.ts b/packages/worker/src/workflow.spec.ts index d5d0ce1d..fff63d3a 100644 --- a/packages/worker/src/workflow.spec.ts +++ b/packages/worker/src/workflow.spec.ts @@ -19,12 +19,12 @@ const contract = defineContract({ processOrder: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), cancelOrder: defineWorkflow({ input: z.object({}), output: z.object({}), - idempotency: "allow-duplicate", + startPolicy: "allow-duplicate", }), }, }); diff --git a/packages/worker/src/workflow.ts b/packages/worker/src/workflow.ts index 805b9dc9..3262f959 100644 --- a/packages/worker/src/workflow.ts +++ b/packages/worker/src/workflow.ts @@ -758,7 +758,7 @@ export type WorkflowContext< * * The `contract` argument is always required — it identifies the task * queue and workflow definition the child runs against, and supplies the - * `workflowIdReusePolicy` from the target workflow's declared `idempotency` + * `workflowIdReusePolicy` from the target workflow's declared `startPolicy` * mode: * - Same-contract child: pass this worker's own contract and one of its * workflow names. @@ -819,7 +819,7 @@ export type WorkflowContext< * * The `contract` argument is always required — it identifies the task * queue and workflow definition the child runs against, and supplies the - * `workflowIdReusePolicy` from the target workflow's declared `idempotency` + * `workflowIdReusePolicy` from the target workflow's declared `startPolicy` * mode: * - Same-contract child: pass this worker's own contract and one of its * workflow names. From 5c47fa18bf0e127fecab01c12ca2ac426e4a71c1 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:15:22 +0200 Subject: [PATCH 05/12] feat(testing): one-call fixture for the time-skipping tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #426. `createContractTest` wired worker + client + contract binding in one call, but only over the testcontainers server; the Docker-free tier handed callers `testRig`, which requires building a `WorkflowBundleWithSourceMap` and holding a `TestWorkflowEnvironment` by hand. So the ergonomic path was also the one that needed a Docker daemon. `createTimeSkippingContractTest` closes that: the environment and bundle are worker-scoped (bundling dominates runtime), the rig is per-test, and `testRig` remains the lower-level seam. A worker a test never ran still holds the environment's native connection, and the env teardown then fails with "Cannot close connection while Workers hold a reference to it" — the fixture releases it with a `runUntil(Promise.resolve())`, since `shutdown()` throws for a worker that is not RUNNING. Verified against a real time-skipping server in `one-call-fixture.inprocess.spec.ts`, which is itself the assertion: it can only run if every piece of the wiring is right. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .changeset/time-skipping-contract-test.md | 14 ++ docs/how-to/test-workflows.md | 37 +++++ packages/testing/src/time-skipping.ts | 130 ++++++++++++++++++ .../one-call-fixture.inprocess.spec.ts | 55 ++++++++ 4 files changed, 236 insertions(+) create mode 100644 .changeset/time-skipping-contract-test.md create mode 100644 packages/worker/src/__tests__/one-call-fixture.inprocess.spec.ts diff --git a/.changeset/time-skipping-contract-test.md b/.changeset/time-skipping-contract-test.md new file mode 100644 index 00000000..ed02fa12 --- /dev/null +++ b/.changeset/time-skipping-contract-test.md @@ -0,0 +1,14 @@ +--- +"@temporal-contract/testing": minor +--- + +`createTimeSkippingContractTest({ contract, workflowsPath, activities })` — the +one-call fixture for the **time-skipping** tier, the Docker-free counterpart to +`createContractTest`. It owns the `TestWorkflowEnvironment`, the workflow bundle +(built once per Vitest worker process), the worker, the `TypedClient` binding, +and the replay-on-finish check, and hands the test `{ worker, client }`. + +Previously the tier with the better ergonomics was also the one that needed +Docker: the time-skipping tier only offered `testRig`, which makes the caller +build a bundle and manage the environment. `testRig` stays as the lower-level +seam. diff --git a/docs/how-to/test-workflows.md b/docs/how-to/test-workflows.md index 1b5eed67..d4fc769d 100644 --- a/docs/how-to/test-workflows.md +++ b/docs/how-to/test-workflows.md @@ -359,6 +359,43 @@ export default createGlobalSetup({ }); ``` +### Wire the whole stack with `createTimeSkippingContractTest` + +The time-skipping tier has the same one-call fixture, with no Docker and no +server to run — reach for this one first, and drop to the Dockerized +`createContractTest` below only for what needs a real cluster (visibility, +search attributes, schedules, retention): + +```typescript +import { createTimeSkippingContractTest } from "@temporal-contract/testing/time-skipping"; +import { workflowsPathFromURL } from "@temporal-contract/worker/worker"; +import { describe, expect } from "vitest"; + +const it = createTimeSkippingContractTest({ + contract: orderContract, + workflowsPath: workflowsPathFromURL(import.meta.url, "./workflows.js"), + activities, +}); + +describe("order processing", () => { + it("processes an order end-to-end", async ({ worker, client }) => { + const result = await worker.raw.runUntil(async () => + client.executeWorkflow("processOrder", { + workflowId: `order-${Date.now()}`, + args: { orderId: "ORD-1" }, + }), + ); + + await expect(result).toBeOk(); + }); +}); +``` + +It owns the environment, the workflow bundle (built once per Vitest worker +process), the worker, the `TypedClient` binding, and the replay-on-finish +check. `testRig` remains the lower-level seam for suites that need to hold +those pieces themselves. + ### Wire the whole stack with `createContractTest` `@temporal-contract/testing/contract` builds a vitest `it` whose fixtures run diff --git a/packages/testing/src/time-skipping.ts b/packages/testing/src/time-skipping.ts index ce047e17..4747a604 100644 --- a/packages/testing/src/time-skipping.ts +++ b/packages/testing/src/time-skipping.ts @@ -40,12 +40,20 @@ * }); * ``` */ +import type { ContractClient } from "@temporal-contract/client"; +import type { ContractDefinition } from "@temporal-contract/contract"; +import type { ActivitiesHandler } from "@temporal-contract/worker/activity"; +import type { TypedWorker } from "@temporal-contract/worker/worker"; import { TestWorkflowEnvironment, type TimeSkippingTestWorkflowEnvironmentOptions, } from "@temporalio/testing"; +import type { WorkflowBundleWithSourceMap } from "@temporalio/worker"; import { it as vitestIt } from "vitest"; +import { testRig } from "./test-rig.js"; +import { bundleFor } from "./workflow-bundle.js"; + /** * Create a time-skipping `TestWorkflowEnvironment` directly — for suites * that prefer explicit `beforeAll`/`afterAll` management over the {@link it} @@ -100,3 +108,125 @@ export function createTimeSkippingTest(options?: TimeSkippingTestWorkflowEnviron * indirection hides that usage from knip. */ export const it = createTimeSkippingTest(); + +/** + * Options for {@link createTimeSkippingContractTest}. + */ +export type CreateTimeSkippingContractTestOptions = { + /** The contract under test — its task queue names the worker's queue. */ + contract: TContract; + /** + * Path to the workflows file to bundle — typically built with + * `workflowsPathFromURL(import.meta.url, "./x.workflows.js")` from + * `@temporal-contract/worker/worker`, or `fixturePath` from + * `@temporal-contract/testing/workflow-bundle`. + */ + workflowsPath: string; + /** Activities handler built with `declareActivitiesHandler`. Omit it for a workflow-only worker. */ + activities?: ActivitiesHandler; + /** + * Workflow-ID prefixes whose executions are deliberately left non-terminal, + * with a reason — forwarded to {@link testRig}, which fails a test that + * leaves an unlisted execution unreplayable. + */ + replaySkipAllowlist?: Readonly>; + /** Forwarded to `TestWorkflowEnvironment.createTimeSkipping`. */ + environment?: TimeSkippingTestWorkflowEnvironmentOptions; +}; + +/** + * The one-call fixture for the **time-skipping** tier: a bundled worker, the + * contract-bound client, and the replay-on-finish check, with no Docker and + * no server to run. + * + * The counterpart to `createContractTest` from + * `@temporal-contract/testing/contract`, which wires the same stack against + * the testcontainers-provided real server. Reach for that one when a test + * needs what only a real cluster has — visibility, search attributes, + * schedules, retention; reach for this one for everything else, which is + * most workflow tests. + * + * The environment and the workflow bundle are **worker-scoped** (built once + * per Vitest worker process, since bundling dominates the runtime); the + * worker and client are per-test. + * + * Like {@link testRig}, this deliberately does **not** scope the task queue: + * a same-workflow continue-as-new must land on the contract's static queue, + * because the contract is closed over inside the bundled workflow module and + * a test-side copy can never reach it. Suites needing isolation keep calling + * `withTaskQueue` themselves. + * + * @example + * ```ts + * import { createTimeSkippingContractTest } from "@temporal-contract/testing/time-skipping"; + * import { workflowsPathFromURL } from "@temporal-contract/worker/worker"; + * import { describe, expect } from "vitest"; + * + * const it = createTimeSkippingContractTest({ + * contract: orderContract, + * workflowsPath: workflowsPathFromURL(import.meta.url, "./order.workflows.js"), + * activities, + * }); + * + * describe("order processing", () => { + * it("processes an order end-to-end", async ({ client }) => { + * const result = await client.executeWorkflow("processOrder", { + * workflowId: `order-${Date.now()}`, + * args: { orderId: "ORD-1" }, + * }); + * await expect(result).toBeOk(); + * }); + * }); + * ``` + */ +export function createTimeSkippingContractTest( + options: CreateTimeSkippingContractTestOptions, +) { + return createTimeSkippingTest(options.environment).extend<{ + bundle: WorkflowBundleWithSourceMap; + rig: { worker: TypedWorker; client: ContractClient }; + worker: TypedWorker; + client: ContractClient; + }>({ + bundle: [ + // oxlint-disable-next-line no-empty-pattern + async ({}, use) => { + await use(await bundleFor(options.workflowsPath)); + }, + { scope: "worker" }, + ], + // One rig per test: `testRig` builds the worker and the client together + // (the client is a Proxy over the bound one, recording started IDs for + // the replay-on-finish check), so they cannot be built independently. + rig: async ({ testEnv, bundle }, use) => { + const rig = await testRig(testEnv, { + contract: options.contract, + bundle, + ...(options.activities !== undefined ? { activities: options.activities } : {}), + ...(options.replaySkipAllowlist !== undefined + ? { replaySkipAllowlist: options.replaySkipAllowlist } + : {}), + }); + + await use(rig); + + // A worker the test never ran still holds a reference to the + // environment's native connection, and the worker-scoped `testEnv` + // teardown then fails with "Cannot close connection while Workers hold + // a reference to it". `runUntil` on an already-resolved promise starts + // and immediately stops it, which is the only way to release that + // reference from the `INITIALIZED` state (`shutdown()` throws unless + // the worker is `RUNNING`). Tests that used `runUntil` themselves are + // already `STOPPED` and skip this. + if (rig.worker.raw.getState() === "INITIALIZED") { + await rig.worker.raw.runUntil(Promise.resolve()); + } + }, + worker: async ({ rig }, use) => { + await use(rig.worker); + }, + client: async ({ rig }, use) => { + await use(rig.client); + }, + }); +} diff --git a/packages/worker/src/__tests__/one-call-fixture.inprocess.spec.ts b/packages/worker/src/__tests__/one-call-fixture.inprocess.spec.ts new file mode 100644 index 00000000..c0953561 --- /dev/null +++ b/packages/worker/src/__tests__/one-call-fixture.inprocess.spec.ts @@ -0,0 +1,55 @@ +/** + * The one-call time-skipping fixture, exercised against a real time-skipping + * server. + * + * This suite IS the assertion: everything the sibling `*.inprocess.spec.ts` + * files wire by hand — the environment, the workflow bundle, the worker, the + * `TypedClient` + contract binding, and the replay-on-finish check — is + * supplied by the single `createTimeSkippingContractTest(...)` call below. If + * any of that wiring is wrong, these tests cannot run at all. + */ +import { createTimeSkippingContractTest } from "@temporal-contract/testing/time-skipping"; +import { fixturePath } from "@temporal-contract/testing/workflow-bundle"; +import { OkAsync } from "unthrown"; +import { describe, expect } from "vitest"; + +import { declareActivitiesHandler } from "../activity.js"; +import { inprocessContract } from "./inprocess.contract.js"; + +const activities = declareActivitiesHandler({ + contract: inprocessContract, + activities: { + placeOrder: { + charge: () => OkAsync({ transactionId: "txn-one-call" }), + }, + }, +}); + +const it = createTimeSkippingContractTest({ + contract: inprocessContract, + workflowsPath: fixturePath(import.meta.url, "inprocess.workflows"), + activities, +}); + +describe("createTimeSkippingContractTest", () => { + it("runs a workflow end to end with no Docker and no hand-wiring", async ({ worker, client }) => { + const outcome = await worker.raw.runUntil(async () => + client.executeWorkflow("placeOrder", { + workflowId: `one-call-${Date.now()}`, + args: { orderId: "ORD-1", amount: 42 }, + }), + ); + + expect(outcome).toBeOk(); + }); + + it("hands out a client bound to the contract, so unknown names don't compile", async ({ + client, + }) => { + // @ts-expect-error -- "notOnThisContract" is not a workflow of inprocessContract + const bad = () => client.startWorkflow("notOnThisContract", { workflowId: "x", args: {} }); + void bad; + + expect(typeof client.executeWorkflow).toBe("function"); + }); +}); From 54c3cdee054eb6bfb80c39d7cc3829a6fcd37ff6 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:24:05 +0200 Subject: [PATCH 06/12] docs(examples): use the ergonomics the packages ship MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #422. The order-processing sample predated most of the library's own helpers and hand-rolled them instead — the thing new users copy from taught the long way round, and in one place the untyped one. - `integration.spec.ts` drops ~40 lines of fixture wiring (including a raw `Worker.create` from `@temporalio/worker`, not `TypedWorker`) for `createContractTest`. - The three best-effort notification folds become `bestEffort` calls (#420). - The `if ("status" in paymentOutcome)` shape-sniff becomes a tagged fold — the two arms mean different things and now say so. - The client example swaps eleven imported tag constants for the shipped pattern groups (#421). - `processOrder` derives its workflow ID (#429) and `processPayment` / `refundPayment` declare idempotency keys (#428), so the contract's 25-line comment conceding a double-charge window is replaced by a fix: the mock gateway now keeps a settled-charge ledger keyed on the declared key, which is what makes the example demonstrate the at-least-once guarantee instead of describing it. Correction to the issue: `workflowsPathFromURL` does NOT replace the examples' hand-rolled `workflowPath`. It takes the extension literally, and these samples run from `.ts` source. The spec uses `fixturePath` (which derives the caller's extension) and the runtime worker keeps `extname(import.meta.url)`, now spelled through the shipped helper with a comment saying why. `getHandle` still addresses executions by raw ID, so for a workflow whose ID the contract derives, the tests read it off the start result rather than re-deriving it. The example's vitest config gains a workspace-only alias + `server.deps.inline` for `@temporal-contract/testing`, whose peers cannot resolve through pnpm's symlink from a nested example — the same technique, and the same reason, as `packages/worker/vitest.config.ts`. Verified: 8/8 integration tests pass against a real Dockerized Temporal. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .../order-processing-client/src/client.ts | 102 ++++++--------- .../order-processing-contract/src/contract.ts | 50 ++++---- .../src/application/activities.ts | 14 ++- .../src/application/worker.ts | 13 +- .../src/application/workflows.ts | 117 ++++++------------ .../src/domain/ports/payment.port.ts | 17 ++- .../usecases/process-payment.usecase.ts | 8 +- .../domain/usecases/refund-payment.usecase.ts | 4 +- .../adapters/payment.adapter.ts | 36 +++++- .../src/integration.spec.ts | 106 +++++----------- .../order-processing-worker/vitest.config.ts | 33 +++++ 11 files changed, 235 insertions(+), 265 deletions(-) diff --git a/examples/order-processing-client/src/client.ts b/examples/order-processing-client/src/client.ts index 43f8c255..76e23e15 100644 --- a/examples/order-processing-client/src/client.ts +++ b/examples/order-processing-client/src/client.ts @@ -1,15 +1,15 @@ import { - SCHEDULE_NOT_FOUND_ERROR_TAG, + SCHEDULE_CREATE_PATTERNS, SCHEDULE_ALREADY_EXISTS_ERROR_TAG, - SIGNAL_VALIDATION_ERROR_TAG, + SCHEDULE_NOT_FOUND_ERROR_TAG, + SIGNAL_PATTERNS, TypedClient, WORKFLOW_ALREADY_STARTED_ERROR_TAG, - WORKFLOW_CANCELLED_ERROR_TAG, + WORKFLOW_EXECUTE_PATTERNS, WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG, WORKFLOW_FAILED_ERROR_TAG, - WORKFLOW_NOT_IN_CONTRACT_ERROR_TAG, - WORKFLOW_TERMINATED_ERROR_TAG, - WORKFLOW_TIMEOUT_ERROR_TAG, + WORKFLOW_RESULT_PATTERNS, + WORKFLOW_STOPPED_PATTERNS, WORKFLOW_VALIDATION_ERROR_TAG, } from "@temporal-contract/client"; import { @@ -81,10 +81,10 @@ async function run() { totalAmount: 149.97, // above the worker's $100 approval threshold }; - const startResult = await orders.startWorkflow("processOrder", { - workflowId: approvalOrder.orderId, - args: approvalOrder, - }); + // No `workflowId` here: the contract derives it (`order-${orderId}`), so a + // caller cannot accidentally defeat the workflow's `startPolicy` by passing + // a fresh ID per attempt. Supplying one is a type error. + const startResult = await orders.startWorkflow("processOrder", { args: approvalOrder }); if (!startResult.isOk()) { logger.error( { err: startResult.isErr() ? startResult.error : startResult.cause }, @@ -109,14 +109,12 @@ async function run() { }); approvalSent.match({ ok: () => logger.info("✍️ Approval signal sent"), + // One group instead of every signal tag by hand — still exhaustive: a + // member missing from the arm is a compile error naming it. errCases: (matcher) => - matcher - .with(P.tag(SIGNAL_VALIDATION_ERROR_TAG), (err) => - logger.error({ error: err }, "❌ Signal payload rejected by the contract"), - ) - .with(P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), (err) => - logger.error({ error: err }, "❌ Workflow execution not found"), - ), + matcher.with(...SIGNAL_PATTERNS, (err) => + logger.error({ error: err }, "❌ Signal could not be delivered"), + ), defect: (cause) => logger.error({ cause }, "❌ Unexpected failure sending signal"), }); @@ -142,14 +140,8 @@ async function run() { // Everything else the result phase can surface — validation, generic // failure, the first-class outcome trio (cancelled/terminated/timed // out), and a missing execution. - .with( - P.tag(WORKFLOW_VALIDATION_ERROR_TAG), - P.tag(WORKFLOW_FAILED_ERROR_TAG), - P.tag(WORKFLOW_CANCELLED_ERROR_TAG), - P.tag(WORKFLOW_TERMINATED_ERROR_TAG), - P.tag(WORKFLOW_TIMEOUT_ERROR_TAG), - P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), - (err) => logger.error({ error: err }, "❌ Workflow did not complete successfully"), + .with(...WORKFLOW_RESULT_PATTERNS, (err) => + logger.error({ error: err }, "❌ Workflow did not complete successfully"), ), defect: (cause) => logger.error({ cause }, "❌ Unexpected failure awaiting result"), }); @@ -166,10 +158,7 @@ async function run() { totalAmount: 119.97, // above the threshold — waits for approval, giving us time to cancel }; - const cancelStart = await orders.startWorkflow("processOrder", { - workflowId: cancelOrder.orderId, - args: cancelOrder, - }); + const cancelStart = await orders.startWorkflow("processOrder", { args: cancelOrder }); if (!cancelStart.isOk()) { logger.error( { err: cancelStart.isErr() ? cancelStart.error : cancelStart.cause }, @@ -181,7 +170,10 @@ async function run() { // `getHandle` is synchronous: the only failure mode is a workflow name // missing from the contract, surfaced as a sync `Result` Err. Whether the // *execution* exists is answered lazily by the handle's methods. - const fetchedHandle = orders.getHandle("processOrder", cancelOrder.orderId); + // The ID came from the contract's derivation, so read it off the start + // result rather than re-deriving it here — the derivation lives in one + // place on purpose. + const fetchedHandle = orders.getHandle("processOrder", cancelStart.value.workflowId); if (!fetchedHandle.isOk()) { logger.error( { err: fetchedHandle.isErr() ? fetchedHandle.error : fetchedHandle.cause }, @@ -198,14 +190,12 @@ async function run() { const cancelSent = await cancelHandle.signals.cancelRequested(); cancelSent.match({ ok: () => logger.info("🛑 Cancellation signal sent"), + // One group instead of every signal tag by hand — still exhaustive: a + // member missing from the arm is a compile error naming it. errCases: (matcher) => - matcher - .with(P.tag(SIGNAL_VALIDATION_ERROR_TAG), (err) => - logger.error({ error: err }, "❌ Signal payload rejected by the contract"), - ) - .with(P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), (err) => - logger.error({ error: err }, "❌ Workflow execution not found"), - ), + matcher.with(...SIGNAL_PATTERNS, (err) => + logger.error({ error: err }, "❌ Signal could not be delivered"), + ), defect: (cause) => logger.error({ cause }, "❌ Unexpected failure sending signal"), }); @@ -224,11 +214,8 @@ async function run() { // The first-class outcome errors get their own arm here: a // server-side cancel / terminate / timeout is a distinct outcome, // not a generic "failure" — no `err.cause instanceof ...` digging. - .with( - P.tag(WORKFLOW_CANCELLED_ERROR_TAG), - P.tag(WORKFLOW_TERMINATED_ERROR_TAG), - P.tag(WORKFLOW_TIMEOUT_ERROR_TAG), - (err) => logger.warn({ error: err }, `🛑 Workflow ${err.name}: execution was stopped`), + .with(...WORKFLOW_STOPPED_PATTERNS, (err) => + logger.warn({ error: err }, `🛑 Workflow ${err.name}: execution was stopped`), ) .with(P.tag(WORKFLOW_VALIDATION_ERROR_TAG), (err) => logger.error({ error: err }, "❌ Workflow output validation failed"), @@ -257,10 +244,7 @@ async function run() { // `executeWorkflow` combines start + result, so its error union is the // widest: every modeled tag (package-namespaced `@temporal-contract/...`) // plus `Ok` and `Defect` must be handled, or it is a compile error. - const executeResult = await orders.executeWorkflow("processOrder", { - workflowId: quickOrder.orderId, - args: quickOrder, - }); + const executeResult = await orders.executeWorkflow("processOrder", { args: quickOrder }); executeResult.match({ ok: (output) => { @@ -292,17 +276,12 @@ async function run() { .with(P.tag(WORKFLOW_ALREADY_STARTED_ERROR_TAG), (err) => logger.warn({ error: err }, "⏭️ Workflow already started — skipping"), ) - // Everything else executeWorkflow can err with — the remaining - // start-phase and result-phase members. - .with( - P.tag(WORKFLOW_NOT_IN_CONTRACT_ERROR_TAG), - P.tag(WORKFLOW_VALIDATION_ERROR_TAG), - P.tag(WORKFLOW_FAILED_ERROR_TAG), - P.tag(WORKFLOW_CANCELLED_ERROR_TAG), - P.tag(WORKFLOW_TERMINATED_ERROR_TAG), - P.tag(WORKFLOW_TIMEOUT_ERROR_TAG), - P.tag(WORKFLOW_EXECUTION_NOT_FOUND_ERROR_TAG), - (err) => logger.error({ error: err }, "❌ Order processing failed"), + // Everything else executeWorkflow can err with. The group names both + // phases including `WorkflowAlreadyStartedError`; the arm above + // already subtracted that one, so listing it again matches nothing + // and costs nothing. + .with(...WORKFLOW_EXECUTE_PATTERNS, (err) => + logger.error({ error: err }, "❌ Order processing failed"), ), // A defect is an unmodeled failure (a bug) — including technical/ // infrastructure faults like a dropped connection (a `RuntimeClientError` @@ -335,12 +314,9 @@ async function run() { logger.info({ scheduleId: err.scheduleId }, "⏭️ Schedule already exists — reusing it"); return orders.schedule.getHandle(err.scheduleId); }) - .with(P.tag(WORKFLOW_NOT_IN_CONTRACT_ERROR_TAG), (err) => { - logger.error({ error: err }, "❌ Workflow not declared in the contract"); - return undefined; - }) - .with(P.tag(WORKFLOW_VALIDATION_ERROR_TAG), (err) => { - logger.error({ error: err }, "❌ Schedule args rejected by the contract"); + // The rest of what `schedule.create` can produce, as one group. + .with(...SCHEDULE_CREATE_PATTERNS, (err) => { + logger.error({ error: err }, "❌ Schedule could not be created"); return undefined; }), defect: (cause) => { diff --git a/examples/order-processing-contract/src/contract.ts b/examples/order-processing-contract/src/contract.ts index 0bbdf42a..19cd6f4a 100644 --- a/examples/order-processing-contract/src/contract.ts +++ b/examples/order-processing-contract/src/contract.ts @@ -90,6 +90,11 @@ const purgeExpiredOrders = defineActivity({ const processPayment = defineActivity({ input: z.object({ customerId: z.string(), amount: z.number() }), output: PaymentResultSchema, + // Temporal runs an activity AT LEAST once — a retry, a worker crash, or a + // completion that succeeded but was never recorded all re-run this. The + // key travels to the gateway so the second run settles the first charge + // instead of making a new one. + idempotencyKey: ({ customerId, amount }) => `charge:${customerId}:${amount}`, errors: { PaymentDeclined: paymentDeclinedError, }, @@ -125,6 +130,10 @@ const createShipment = defineActivity({ const refundPayment = defineActivity({ input: z.string(), output: z.void(), + // Same reasoning as `processPayment`, opposite direction — and a distinct + // prefix, because a gateway keyed on the transaction alone would treat the + // charge and its refund as the same request. + idempotencyKey: (transactionId) => `refund:${transactionId}`, }); // ============================================================================ @@ -163,31 +172,24 @@ const getOrderStatus = defineQuery({ output: OrderStatusReportSchema }); const processOrder = defineWorkflow({ input: OrderSchema, output: OrderResultSchema, - // A Completed run charged the customer — Temporal's default - // (`allow-duplicate`) would let a retried start under the same order ID - // charge them again. `retry-if-failed` blocks that while still letting a - // start be retried after a run that ended Failed *before* any charge went - // through — chiefly `PaymentDeclined` (see this contract's implementation, - // `order-processing-worker/src/application/workflows.ts`), where the - // customer would otherwise have to be given a new order ID to try again. + // The ID is derived from the order, not supplied by the caller: a client + // passing a fresh UUID per attempt would make any start policy inert, + // because every retry would be a different workflow ID. + workflowId: ({ orderId }) => `order-${orderId}`, + // A Completed run charged the customer, so a second successful run under + // the same order must not happen. `retry-if-failed` still allows a start + // after a run that ended Failed — chiefly `PaymentDeclined`, where no + // charge went through, so the customer can retry without a new order ID. // - // Caveat this example doesn't fully close: several post-charge paths in - // that file also end the run in a state `retry-if-failed` treats as - // re-runnable (`ALLOW_DUPLICATE_FAILED_ONLY` covers Cancelled/Terminated/ - // TimedOut, not just Failed) — including a failed compensating - // `refundPayment` that deliberately fails the workflow with the charge - // unrefunded (`workflows.ts:287-298`), real cancellation during/after - // inventory reservation ending the run Cancelled post-charge - // (`workflows.ts:262`), and a `createShipment` failure, which has no - // rollback path and is left to fail the workflow outright - // (`workflows.ts:350-358`). A retried start after any of these would - // re-enter `processPayment` and double-charge — this list is illustrative, - // not exhaustive; any future terminal failure after payment has the same - // shape unless it's explicitly compensated. `once-per-id` would close the - // gap entirely, at the cost of forcing a fresh workflow ID for every - // legitimate retry, including the common pre-charge `PaymentDeclined` case - // above — kept as `retry-if-failed` here because that trade favors the - // common case, not because the gap doesn't exist. + // Post-charge terminal failures (a failed compensating `refundPayment`, a + // cancel during inventory reservation, a `createShipment` failure) also end + // the run in a state this policy treats as re-runnable. What stops those + // from double-charging is not this field but `processPayment`'s + // `idempotencyKey`: a retried start derives the same key from the same + // customer and amount, so the gateway settles one charge no matter how + // many times the activity runs. Start policy dedupes *executions*; the + // activity key dedupes *effects*, and Temporal's at-least-once activity + // guarantee means only the second one can close this gap. startPolicy: "retry-if-failed", activities: { processPayment, diff --git a/examples/order-processing-worker/src/application/activities.ts b/examples/order-processing-worker/src/application/activities.ts index 038af616..c1a19398 100644 --- a/examples/order-processing-worker/src/application/activities.ts +++ b/examples/order-processing-worker/src/application/activities.ts @@ -89,9 +89,15 @@ export const activities = declareActivitiesHandler({ // an `ApplicationFailure(type: "PaymentDeclined")` and rehydrates as a // typed `ContractError` on the workflow side. Only gateway faults ride // the generic `ApplicationFailure` path. - processPayment: ({ errors, input: { customerId, amount } }) => + // `idempotencyKey` is declared on this activity in the contract, so it + // arrives typed as `string` (an activity without one types it + // `undefined`, so reaching for a key that was never declared is a + // compile error). It goes to the gateway, which is what makes a + // re-run under Temporal's at-least-once guarantee settle the first + // charge instead of making a second one. + processPayment: ({ errors, idempotencyKey, input: { customerId, amount } }) => fromPromise( - processPaymentUseCase.execute(customerId, amount), + processPaymentUseCase.execute(customerId, amount, idempotencyKey), qualifyFailure("PAYMENT_GATEWAY_ERROR", { expected: PaymentError, message: "Payment gateway call failed", @@ -130,9 +136,9 @@ export const activities = declareActivitiesHandler({ }), ), - refundPayment: ({ input: transactionId }) => + refundPayment: ({ idempotencyKey, input: transactionId }) => fromPromise( - refundPaymentUseCase.execute(transactionId), + refundPaymentUseCase.execute(transactionId, idempotencyKey), qualifyFailure("REFUND_FAILED", { expected: PaymentError, message: "Refund failed" }), ), }, diff --git a/examples/order-processing-worker/src/application/worker.ts b/examples/order-processing-worker/src/application/worker.ts index cd345b7e..ab120640 100644 --- a/examples/order-processing-worker/src/application/worker.ts +++ b/examples/order-processing-worker/src/application/worker.ts @@ -1,17 +1,12 @@ import { extname } from "node:path"; -import { fileURLToPath } from "node:url"; import { orderProcessingContract } from "@temporal-contract/sample-order-processing-contract"; -import { TypedWorker } from "@temporal-contract/worker/worker"; +import { TypedWorker, workflowsPathFromURL } from "@temporal-contract/worker/worker"; import { NativeConnection } from "@temporalio/worker"; import { logger } from "../logger.js"; import { activities } from "./activities.js"; -function workflowPath(filename: string): string { - return fileURLToPath(new URL(`./${filename}${extname(import.meta.url)}`, import.meta.url)); -} - /** * Start the Temporal Worker * @@ -35,7 +30,11 @@ async function run() { contract: orderProcessingContract, connection, namespace: "default", - workflowsPath: workflowPath("workflows"), + // This sample runs straight from TypeScript source under `tsx`, so the + // sibling module is `workflows.ts` here and `workflows.js` once built — + // hence `extname(import.meta.url)` rather than a literal `.js`. An app + // that only ever runs built output writes `"./workflows.js"`. + workflowsPath: workflowsPathFromURL(import.meta.url, `./workflows${extname(import.meta.url)}`), activities, }); if (workerResult.isDefect()) { diff --git a/examples/order-processing-worker/src/application/workflows.ts b/examples/order-processing-worker/src/application/workflows.ts index 52e4028b..8f6a5a06 100644 --- a/examples/order-processing-worker/src/application/workflows.ts +++ b/examples/order-processing-worker/src/application/workflows.ts @@ -5,6 +5,7 @@ import { import { ACTIVITY_CANCELLED_ERROR_TAG, ACTIVITY_ERROR_TAG, + bestEffort, declareWorkflow, propagateFailure, rethrowCancellation, @@ -155,7 +156,7 @@ export const processOrder = declareWorkflow({ const paymentOutcome = await activities .processPayment({ customerId: order.customerId, amount: order.totalAmount }) .match({ - ok: (payment) => payment, + ok: (payment) => ({ kind: "paid" as const, payment }), errCases: (matcher) => matcher // The only declared error on `processPayment` — the object @@ -165,36 +166,20 @@ export const processOrder = declareWorkflow({ status = "failed"; log.error(`Payment declined for order ${order.orderId}: ${failure.data.reason}`); - // Best-effort notification: the declined-payment outcome below - // is what matters, so an undeclared notification failure only - // gets a warning — it must not swallow (or block) the rethrow. - // Real cancellation is the exception: it must still propagate. - await activities - .sendNotification({ + // Best-effort: the declined-payment outcome below is what + // matters, so a failed notification only earns a warning. It + // must not swallow the rethrow — and `bestEffort` keeps real + // cancellation propagating, which a hand-written fold has to + // remember not to absorb. + await bestEffort( + activities.sendNotification({ customerId: order.customerId, subject: "Order Failed", message: `We're sorry, but your order ${order.orderId} could not be processed. Your payment was declined (${failure.data.reason}).`, - }) - .match({ - ok: () => undefined, - errCases: (matcher) => - matcher - .with(P.tag(ACTIVITY_CANCELLED_ERROR_TAG), (cancelled) => - rethrowCancellation(cancelled), - ) - .with(P.tag(ACTIVITY_ERROR_TAG), (notifyFailure) => { - log.warn( - `Failed to notify customer of declined payment: ${notifyFailure.message}`, - ); - }), - // A defect here is still just a failed email — the - // PaymentDeclined outcome about to be rethrown below is - // already authoritative and must not be blocked by a - // notification bug. - defect: (cause) => { - log.warn(`Failed to notify customer of declined payment: ${cause}`); - }, - }); + }), + (notifyFailure) => + log.warn(`Failed to notify customer of declined payment: ${notifyFailure}`), + ); // Rethrow as this workflow's own declared contract error: the // execution fails with `ApplicationFailure(type: "PaymentDeclined")` @@ -215,10 +200,13 @@ export const processOrder = declareWorkflow({ status = "failed"; log.error(`Payment activity failed for order ${order.orderId}: ${failure.message}`); return { - orderId: order.orderId, - status: "failed" as const, - failureReason: "Payment could not be processed", - errorCode: "PAYMENT_UNAVAILABLE", + kind: "aborted" as const, + output: { + orderId: order.orderId, + status: "failed" as const, + failureReason: "Payment could not be processed", + errorCode: "PAYMENT_UNAVAILABLE", + }, }; }), // Unmodeled failure (a bug, not an anticipated outcome) — rethrow at @@ -229,12 +217,14 @@ export const processOrder = declareWorkflow({ }, }); - if ("status" in paymentOutcome) { - // The fold produced the workflow's failed output — return it as-is. - return paymentOutcome; + // Discriminated by a tag the fold set, not by sniffing for a `status` + // field: the two arms mean different things ("here is the payment" vs + // "here is the workflow's output"), so they say so. + if (paymentOutcome.kind === "aborted") { + return paymentOutcome.output; } - const payment = paymentOutcome; + const { payment } = paymentOutcome; log.info(`Payment successful: ${payment.transactionId}`); // ------------------------------------------------------------------ @@ -305,29 +295,14 @@ export const processOrder = declareWorkflow({ ? `We're sorry, but your order ${order.orderId} could not be processed. Our inventory service is temporarily unavailable. Any charges have been refunded.` : `We're sorry, but your order ${order.orderId} could not be processed. One or more items are out of stock. Any charges have been refunded.`; - await activities - .sendNotification({ + await bestEffort( + activities.sendNotification({ customerId: order.customerId, subject: "Order Failed", message: rollbackMessage, - }) - .match({ - ok: () => undefined, - errCases: (matcher) => - matcher - .with(P.tag(ACTIVITY_CANCELLED_ERROR_TAG), (cancelled) => - rethrowCancellation(cancelled), - ) - .with(P.tag(ACTIVITY_ERROR_TAG), (failure) => { - log.warn(`Failed to notify customer of out-of-stock order: ${failure.message}`); - }), - // A defect here is still just a failed email — the order outcome - // above (and about to be returned below) is already authoritative - // and must not be blocked by a notification bug. - defect: (cause) => { - log.warn(`Failed to notify customer of out-of-stock order: ${cause}`); - }, - }); + }), + (failure) => log.warn(`Failed to notify customer of out-of-stock order: ${failure}`), + ); return { orderId: order.orderId, @@ -363,32 +338,16 @@ export const processOrder = declareWorkflow({ // now returns an `AsyncResult` — including cancellation, via // `ActivityCancelledError` — so narrowing the call's own result is enough // and no longer needs a wrapping `cancellableScope` just to observe it. - await activities - .sendNotification({ + // Step 5: confirmation, best-effort — the order already shipped, so a + // failed email is a warning. Cancellation still propagates. + await bestEffort( + activities.sendNotification({ customerId: order.customerId, subject: "Order Confirmed", message: `Your order ${order.orderId} has been confirmed and will be shipped. Tracking: ${shippingResult.trackingNumber}`, - }) - .match({ - ok: () => undefined, - // Cancellation must propagate — absorbing it here would complete the - // workflow after a cancel request instead of ending it `Cancelled`. - errCases: (matcher) => - matcher - .with(P.tag(ACTIVITY_CANCELLED_ERROR_TAG), (cancelled) => - rethrowCancellation(cancelled), - ) - // Non-critical: the order is already shipped, so even an - // undeclared notification failure is only worth a warning. - .with(P.tag(ACTIVITY_ERROR_TAG), (failure) => { - log.warn(`Failed to send confirmation notification: ${failure.message}`); - }), - // A defect here is still just a failed email — the order already - // completed successfully above, and that outcome is authoritative. - defect: (cause) => { - log.warn(`Failed to send confirmation notification: ${cause}`); - }, - }); + }), + (failure) => log.warn(`Failed to send confirmation notification: ${failure}`), + ); // Success! status = "completed"; diff --git a/examples/order-processing-worker/src/domain/ports/payment.port.ts b/examples/order-processing-worker/src/domain/ports/payment.port.ts index 4fe50fde..1d62c0df 100644 --- a/examples/order-processing-worker/src/domain/ports/payment.port.ts +++ b/examples/order-processing-worker/src/domain/ports/payment.port.ts @@ -8,11 +8,22 @@ export type PaymentPort = { * Process a payment for a customer. Resolves with a domain-level outcome: * approved (with transaction details) or declined (with a reason). * Rejections are reserved for technical gateway faults. + * + * `idempotencyKey` is the gateway's dedupe key (Stripe's + * `Idempotency-Key`, and its equivalents): the same key must settle one + * charge however many times the call is repeated. It comes from the + * activity's contract declaration, so a retried activity — or a retried + * workflow — sends the same one. */ - processPayment(customerId: string, amount: number): Promise; + processPayment( + customerId: string, + amount: number, + idempotencyKey: string, + ): Promise; /** - * Refund a payment transaction + * Refund a payment transaction, keyed for the same reason as + * {@link PaymentPort.processPayment}. */ - refundPayment(transactionId: string): Promise; + refundPayment(transactionId: string, idempotencyKey: string): Promise; }; diff --git a/examples/order-processing-worker/src/domain/usecases/process-payment.usecase.ts b/examples/order-processing-worker/src/domain/usecases/process-payment.usecase.ts index add58355..28b75866 100644 --- a/examples/order-processing-worker/src/domain/usecases/process-payment.usecase.ts +++ b/examples/order-processing-worker/src/domain/usecases/process-payment.usecase.ts @@ -10,7 +10,11 @@ import type { PaymentPort } from "../ports/payment.port.js"; export class ProcessPaymentUseCase { constructor(private readonly paymentPort: PaymentPort) {} - async execute(customerId: string, amount: number): Promise { + async execute( + customerId: string, + amount: number, + idempotencyKey: string, + ): Promise { // Business validation if (amount <= 0) { // oxlint-disable-next-line unthrown/no-throw -- known-technical precondition throw in a plain (non-Result) domain helper, wrapped once at the activity boundary via fromPromise(..., qualifyFailure(...)) @@ -23,6 +27,6 @@ export class ProcessPaymentUseCase { } // Delegate to payment port - return this.paymentPort.processPayment(customerId, amount); + return this.paymentPort.processPayment(customerId, amount, idempotencyKey); } } diff --git a/examples/order-processing-worker/src/domain/usecases/refund-payment.usecase.ts b/examples/order-processing-worker/src/domain/usecases/refund-payment.usecase.ts index 13ad0642..cc79107a 100644 --- a/examples/order-processing-worker/src/domain/usecases/refund-payment.usecase.ts +++ b/examples/order-processing-worker/src/domain/usecases/refund-payment.usecase.ts @@ -9,7 +9,7 @@ import type { PaymentPort } from "../ports/payment.port.js"; export class RefundPaymentUseCase { constructor(private readonly paymentPort: PaymentPort) {} - async execute(transactionId: string): Promise { + async execute(transactionId: string, idempotencyKey: string): Promise { // Business validation if (!transactionId || transactionId.trim() === "") { // oxlint-disable-next-line unthrown/no-throw -- known-technical precondition throw in a plain (non-Result) domain helper, wrapped once at the activity boundary via fromPromise(..., qualifyFailure(...)) @@ -17,6 +17,6 @@ export class RefundPaymentUseCase { } // Delegate to payment port - return this.paymentPort.refundPayment(transactionId); + return this.paymentPort.refundPayment(transactionId, idempotencyKey); } } diff --git a/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts b/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts index 950bbc24..c4b0665b 100644 --- a/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts +++ b/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts @@ -9,9 +9,30 @@ import { logger } from "../../logger.js"; * Concrete implementation of PaymentPort for testing/demo purposes */ export class MockPaymentAdapter implements PaymentPort { - async processPayment(customerId: string, amount: number): Promise { + /** + * Charges already settled, by idempotency key. A real gateway keeps this + * ledger on its side; the mock keeps it here so the example actually + * demonstrates the guarantee instead of just passing the key around. + */ + private readonly settled = new Map(); + + async processPayment( + customerId: string, + amount: number, + idempotencyKey: string, + ): Promise { + const alreadySettled = this.settled.get(idempotencyKey); + if (alreadySettled) { + // This is the at-least-once case: the activity ran before (a retry, a + // worker crash, a completion Temporal never recorded) — or the whole + // workflow was restarted under the same order. Same key, same answer, + // one charge. + logger.info({ idempotencyKey }, `↩️ Replayed settled charge for ${idempotencyKey}`); + return alreadySettled; + } + logger.info( - { customerId, amount }, + { customerId, amount, idempotencyKey }, `💳 Processing payment of $${amount} for customer ${customerId}`, ); @@ -31,6 +52,10 @@ export class MockPaymentAdapter implements PaymentPort { `✅ Payment processed: ${result.transactionId}`, ); + // Only an approval is recorded: a decline settled nothing, so a later + // attempt with the same key is free to be approved. + this.settled.set(idempotencyKey, result); + return result; } else { // A decline is a modeled business outcome, not an exception — the @@ -47,8 +72,11 @@ export class MockPaymentAdapter implements PaymentPort { } } - async refundPayment(transactionId: string): Promise { - logger.info({ transactionId }, `💰 Processing refund for transaction ${transactionId}`); + async refundPayment(transactionId: string, idempotencyKey: string): Promise { + logger.info( + { transactionId, idempotencyKey }, + `💰 Processing refund for transaction ${transactionId}`, + ); // Simulate refund processing with 99% success rate const success = Math.random() > 0.01; diff --git a/examples/order-processing-worker/src/integration.spec.ts b/examples/order-processing-worker/src/integration.spec.ts index 355d2b4e..da9dd979 100644 --- a/examples/order-processing-worker/src/integration.spec.ts +++ b/examples/order-processing-worker/src/integration.spec.ts @@ -1,19 +1,10 @@ -import { extname } from "node:path"; -import { fileURLToPath } from "node:url"; - -import { - ContractError, - TypedClient, - WorkflowValidationError, - type ContractClient, -} from "@temporal-contract/client"; +import { ContractError, WorkflowValidationError } from "@temporal-contract/client"; import { orderProcessingContract, type OrderSchema, } from "@temporal-contract/sample-order-processing-contract"; -import { it as baseIt } from "@temporal-contract/testing/extension"; -import { Client } from "@temporalio/client"; -import { Worker } from "@temporalio/worker"; +import { createContractTest } from "@temporal-contract/testing/contract"; +import { fixturePath } from "@temporal-contract/testing/workflow-bundle"; import { describe, expect, vi, beforeEach } from "vitest"; import type { z } from "zod"; @@ -22,47 +13,14 @@ import { paymentAdapter } from "./dependencies.js"; type Order = z.infer; -const it = baseIt.extend<{ - worker: Worker; - client: ContractClient; -}>({ - worker: [ - async ({ workerConnection }, use) => { - // Create and start worker - const worker = await Worker.create({ - connection: workerConnection, - namespace: "default", - taskQueue: orderProcessingContract.taskQueue, - workflowsPath: workflowPath("application/workflows"), - activities, - }); - - // Start worker in background - worker.run().catch((err) => { - console.error("Worker failed:", err); - }); - - await vi.waitFor(() => worker.getState() === "RUNNING", { interval: 100 }); - - await use(worker); - - worker.shutdown(); - - await vi.waitFor(() => worker.getState() === "STOPPED", { interval: 100 }); - }, - { auto: true }, - ], - client: async ({ clientConnection }, use) => { - const rawClient = new Client({ - connection: clientConnection, - namespace: "default", - }); - // Connection-scoped root (E = never, so `.get()` unwraps directly), - // then bind the contract for the typed, contract-scoped surface. - const typedClient = await TypedClient.create({ client: rawClient }).get(); - - await use(typedClient.for(orderProcessingContract)); - }, +const it = createContractTest({ + contract: orderProcessingContract, + // `fixturePath` derives the extension from the CALLER's URL, so this + // resolves to `.ts` under vitest and `.js` from built output. + // `workflowsPathFromURL` takes the extension literally and is the right + // helper once the workflows really are `.js` on disk. + workflowsPath: fixturePath(import.meta.url, "application/workflows"), + activities, }); describe("Order Processing Workflow - Integration Tests", () => { @@ -97,7 +55,6 @@ describe("Order Processing Workflow - Integration Tests", () => { // WHEN const result = await client.executeWorkflow("processOrder", { - workflowId: order.orderId, args: order, }); @@ -130,7 +87,6 @@ describe("Order Processing Workflow - Integration Tests", () => { // WHEN const handleResult = await client.startWorkflow("processOrder", { - workflowId: order.orderId, args: order, }); @@ -138,7 +94,8 @@ describe("Order Processing Workflow - Integration Tests", () => { expect(handleResult).toBeOk(); if (!handleResult.isOk()) throw new Error("Expected Ok result"); const handle = handleResult.value; - expect(handle.workflowId).toBe(order.orderId); + // The contract derived it: `order-${orderId}`. + expect(handle.workflowId).toBe(`order-${order.orderId}`); const result = await handle.result(); expect(result).toBeOk(); @@ -168,19 +125,21 @@ describe("Order Processing Workflow - Integration Tests", () => { }; // WHEN - await client.startWorkflow("processOrder", { - workflowId: order.orderId, - args: order, - }); + const started = await client.startWorkflow("processOrder", { args: order }); + expect(started).toBeOk(); + if (!started.isOk()) throw new Error("Expected Ok result"); // THEN — getHandle is synchronous: the only failure mode is a workflow - // name missing from the contract, surfaced as a sync Result Err. - const handleResult = client.getHandle("processOrder", order.orderId); + // name missing from the contract, surfaced as a sync Result Err. It + // addresses an execution by ID, so for a workflow whose ID the contract + // derives, read that ID off the start result rather than re-deriving it. + const handleResult = client.getHandle("processOrder", started.value.workflowId); expect(handleResult).toBeOk(); if (!handleResult.isOk()) throw new Error("Expected Ok result"); const handle = handleResult.value; - expect(handle.workflowId).toBe(order.orderId); + // The contract derived it: `order-${orderId}`. + expect(handle.workflowId).toBe(`order-${order.orderId}`); const result = await handle.result(); expect(result).toBeOk(); @@ -211,7 +170,6 @@ describe("Order Processing Workflow - Integration Tests", () => { // WHEN const handleResult = await client.startWorkflow("processOrder", { - workflowId: order.orderId, args: order, }); @@ -224,7 +182,7 @@ describe("Order Processing Workflow - Integration Tests", () => { expect(describeResult).toBeOk(); if (describeResult.isOk()) { expect(describeResult.value).toEqual( - expect.objectContaining({ workflowId: order.orderId, type: "processOrder" }), + expect.objectContaining({ workflowId: `order-${order.orderId}`, type: "processOrder" }), ); } @@ -248,7 +206,6 @@ describe("Order Processing Workflow - Integration Tests", () => { // WHEN const execution = await client.executeWorkflow("processOrder", { - workflowId: invalidOrder.orderId, args: invalidOrder as Order, }); @@ -288,7 +245,6 @@ describe("Order Processing Workflow - Integration Tests", () => { }; const handleResult = await client.startWorkflow("processOrder", { - workflowId: order.orderId, args: order, }); expect(handleResult).toBeOk(); @@ -338,12 +294,13 @@ describe("Order Processing Workflow - Integration Tests", () => { totalAmount: 199.99, }; - await client.startWorkflow("processOrder", { - workflowId: order.orderId, - args: order, - }); + const started = await client.startWorkflow("processOrder", { args: order }); + expect(started).toBeOk(); + if (!started.isOk()) throw new Error("Expected Ok result"); - const handleResult = client.getHandle("processOrder", order.orderId); + // The ID came from the contract's derivation — take it from the start + // result instead of re-deriving it at the call site. + const handleResult = client.getHandle("processOrder", started.value.workflowId); expect(handleResult).toBeOk(); if (!handleResult.isOk()) throw new Error("Expected Ok result"); const handle = handleResult.value; @@ -390,7 +347,6 @@ describe("Order Processing Workflow - Integration Tests", () => { // WHEN const result = await client.executeWorkflow("processOrder", { - workflowId: order.orderId, args: order, }); @@ -407,7 +363,3 @@ describe("Order Processing Workflow - Integration Tests", () => { } }); }); - -function workflowPath(filename: string): string { - return fileURLToPath(new URL(`./${filename}${extname(import.meta.url)}`, import.meta.url)); -} diff --git a/examples/order-processing-worker/vitest.config.ts b/examples/order-processing-worker/vitest.config.ts index c5aa6137..5a6ce10c 100644 --- a/examples/order-processing-worker/vitest.config.ts +++ b/examples/order-processing-worker/vitest.config.ts @@ -1,7 +1,40 @@ +import { fileURLToPath } from "node:url"; + import { defineConfig } from "vitest/config"; +const pkg = (path: string) => fileURLToPath(new URL(path, import.meta.url)); + +// Workspace-only plumbing — a real consumer needs none of this. +// +// `@temporal-contract/testing`'s built `contract.mjs` imports `TypedClient` +// from `@temporal-contract/client` and `TypedWorker` from +// `@temporal-contract/worker/worker`, both `peerDependencies` of `testing` +// rather than dependencies. For someone who installed from npm those resolve +// fine: their own `node_modules` sits above the dist file. Inside this +// workspace pnpm symlinks `@temporal-contract/testing` straight to +// `packages/testing`, and Node resolves bare specifiers from that real path, +// which has no route to `client`/`worker`. +// +// So: alias the two specifiers to source, and `server.deps.inline` the +// prebuilt `testing` package so Vitest routes it through Vite's resolver +// (which is what makes the alias apply) instead of externalizing it to +// Node's loader. Same technique, same reason, as +// `packages/worker/vitest.config.ts`. export default defineConfig({ + resolve: { + alias: [ + { + find: /^@temporal-contract\/client$/, + replacement: pkg("../../packages/client/src/index.ts"), + }, + { + find: /^@temporal-contract\/worker\/worker$/, + replacement: pkg("../../packages/worker/src/worker.ts"), + }, + ], + }, test: { + server: { deps: { inline: [/@temporal-contract\/testing/] } }, globalSetup: "@temporal-contract/testing/global-setup", reporters: ["default"], setupFiles: ["./src/vitest.setup.ts"], From f163d05e22dbfae12ab3b3ad52a86f64099cd748 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:25:50 +0200 Subject: [PATCH 07/12] docs: one home for "setup failures are defects, so .get() is safe" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #424. `TypedClient.create`, `TypedWorker.create` and `worker.run()` have an empty Err channel, and four call sites in this repo each re-explained why in their own paragraph — the codebase saying the explanation had no home. It now lives in `the-result-model.md` ("Setup calls have an empty Err channel"), covering both the `.get()` form and the `.isDefect()`-then-exit form, and stating the boundary: this is the ONE place `.get()` is safe by construction. The four copies become one-line pointers, and `TypedWorker.create`'s JSDoc says it on hover. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- docs/explanation/the-result-model.md | 36 +++++++++++++++++++ .../order-processing-client/src/client.ts | 8 ++--- .../src/application/worker.ts | 12 +++---- packages/testing/src/contract.ts | 4 +-- packages/testing/src/test-rig.ts | 3 ++ packages/worker/src/worker.ts | 9 +++++ 6 files changed, 58 insertions(+), 14 deletions(-) diff --git a/docs/explanation/the-result-model.md b/docs/explanation/the-result-model.md index 4e391d31..dfac48b1 100644 --- a/docs/explanation/the-result-model.md +++ b/docs/explanation/the-result-model.md @@ -60,6 +60,42 @@ anticipated failure modes_. Everything that can go wrong is a defect. const client = await TypedClient.create({ client: temporalClient }).get(); ``` +## Setup calls have an empty Err channel + +`TypedClient.create` and `TypedWorker.create` return an `AsyncResult` whose +error type is `never`, and `worker.run()` does the same. That is not an +oversight: **nothing about creating a client or a worker is a modeled domain +outcome.** A bad address, a namespace that does not exist, a server too old to +serve the Schedule API — these are technical faults, and this library routes +technical faults to the defect channel (see above). There is no `Err` case to +name, so `E` is `never`. + +The practical consequence is that `.get()` is the right way to read them: + +```typescript +// E is `never`, so `.get()` unwraps the value directly. A setup defect +// rethrows its cause — which is what you want at process start. +const typedClient = await TypedClient.create({ client: rawClient }).get(); +const worker = await TypedWorker.create({ contract, connection, ... }).get(); +``` + +Reach for `.isDefect()` first only when the process wants to report the +failure itself before exiting: + +```typescript +const created = await TypedWorker.create({ contract, connection, ... }); +if (created.isDefect()) { + logger.error({ err: created.cause }, "worker creation failed"); + process.exit(1); +} +const worker = created.get(); +``` + +This is the one place `.get()` is safe by construction. Everywhere else — +`startWorkflow`, an activity call, `handle.result()` — the Err channel is +populated with outcomes the contract actually models, and `.get()` would throw +away exactly the information the Result exists to carry. Narrow those. + ## The shapes at each boundary This is the table to internalize: diff --git a/examples/order-processing-client/src/client.ts b/examples/order-processing-client/src/client.ts index 76e23e15..0acdf60f 100644 --- a/examples/order-processing-client/src/client.ts +++ b/examples/order-processing-client/src/client.ts @@ -55,11 +55,9 @@ async function run() { namespace: "default", }); - // Connection-scoped root — create once at process start. Creation failures - // (bad connection, missing Schedule API) are technical faults that ride the - // defect channel (a `TechnicalError` cause), so the Err channel is empty - // (`never`) and `.get()` unwraps directly (a setup defect rethrows its - // cause). + // Connection-scoped root — create once at process start. Its Err channel is + // empty (`never`), so `.get()` unwraps directly; see "Setup calls have an + // empty Err channel" in docs/explanation/the-result-model.md. const typedClient = await TypedClient.create({ client: rawClient }).get(); // Contract-scoped client — binding a contract is synchronous, infallible, diff --git a/examples/order-processing-worker/src/application/worker.ts b/examples/order-processing-worker/src/application/worker.ts index ab120640..1c975b42 100644 --- a/examples/order-processing-worker/src/application/worker.ts +++ b/examples/order-processing-worker/src/application/worker.ts @@ -23,9 +23,9 @@ async function run() { address: "localhost:7233", }); - // Create and run the worker via the TypedWorker.create factory — creation - // failures are technical faults that ride the defect channel (a - // TechnicalError cause), not the Err channel and not thrown. + // Creation failures ride the defect channel, not the Err channel and not a + // throw — see "Setup calls have an empty Err channel" in + // docs/explanation/the-result-model.md. const workerResult = await TypedWorker.create({ contract: orderProcessingContract, connection, @@ -41,14 +41,12 @@ async function run() { logger.error({ err: workerResult.cause }, "❌ Worker creation failed"); process.exit(1); } - // The Err channel is empty (never) and the defect case exited above, so - // `.get()` unwraps directly. const worker = workerResult.get(); logger.info("✅ Worker registered successfully"); - // Run the worker loop. `run()` returns AsyncResult — a - // runtime failure is a defect whose cause `.get()` rethrows below. + // `run()` is `AsyncResult` for the same reason: a runtime + // failure is a defect, and `.get()` rethrows its cause. await worker.run().get(); } diff --git a/packages/testing/src/contract.ts b/packages/testing/src/contract.ts index 4b4b3f08..5479d394 100644 --- a/packages/testing/src/contract.ts +++ b/packages/testing/src/contract.ts @@ -112,8 +112,8 @@ export function createContractTest( return baseIt.extend>({ worker: [ async ({ workerConnection }, use) => { - // Technical creation failures ride the defect channel (E = never); - // `get()` unwraps directly and rethrows a defect's cause. + // E is `never` here — see "Setup calls have an empty Err channel" in + // docs/explanation/the-result-model.md. const worker = await TypedWorker.create({ contract, connection: workerConnection, diff --git a/packages/testing/src/test-rig.ts b/packages/testing/src/test-rig.ts index 5b9a7558..ebb28d0f 100644 --- a/packages/testing/src/test-rig.ts +++ b/packages/testing/src/test-rig.ts @@ -196,6 +196,9 @@ export async function testRig( ): Promise<{ worker: TypedWorker; client: ContractClient }> { const { contract, bundle, activities, replaySkipAllowlist = {} } = options; + // `TypedWorker.create`/`TypedClient.create` have an empty Err channel — see + // "Setup calls have an empty Err channel" in + // docs/explanation/the-result-model.md. const worker = await TypedWorker.create({ contract, connection: testEnv.nativeConnection, diff --git a/packages/worker/src/worker.ts b/packages/worker/src/worker.ts index f50ace83..34212b25 100644 --- a/packages/worker/src/worker.ts +++ b/packages/worker/src/worker.ts @@ -221,6 +221,15 @@ export class TypedWorker { * * await worker.run().get(); * ``` + * + * @remarks + * The Err channel is empty (`never`): nothing about creating a worker is a + * modeled domain outcome, so a bad connection or an invalid contract is a + * technical fault on the **defect** channel. `.get()` therefore unwraps + * directly and rethrows a setup defect's cause; narrow with `.isDefect()` + * first only to report the failure before exiting. `run()` is `never` in the + * same way. See "Setup calls have an empty Err channel" in + * `docs/explanation/the-result-model.md`. */ static create( options: CreateWorkerOptions, From 5157bafddae46f729954fcc43719686cf5b98f08 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:27:51 +0200 Subject: [PATCH 08/12] docs: publish a samples-typescript coverage matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #425. `EXAMPLES.md` maps every sample in temporalio/samples-typescript to how temporal-contract expresses it and — for a ✅ — the test that proves it. A doc page does not earn a ✅. Four states rather than two, because "supported" and "tested" are not the same claim: ✅ test-backed, ⚠️ supported but untested here, ❌ unsupported, ➖ not applicable to a contract layer. Writing it was the audit it was meant to be. Two findings worth their own attention: schedules and search attributes both have a typed surface and unit tests against a stubbed client, and neither has a real-server test — which is exactly the tier those two need, since visibility and schedule semantics are what a real cluster does differently. Nexus is the one flat no, four rows wide. Every linked path was checked to exist, and the claims spot-checked against the tests' contents (the dependency-injection row was corrected to the files that actually cover `createContext`). Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- EXAMPLES.md | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 2 +- 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 EXAMPLES.md diff --git a/EXAMPLES.md b/EXAMPLES.md new file mode 100644 index 00000000..31542363 --- /dev/null +++ b/EXAMPLES.md @@ -0,0 +1,115 @@ +# Sample coverage + +What temporal-contract can express, measured against +[temporalio/samples-typescript](https://github.com/temporalio/samples-typescript) +— the closest thing the ecosystem has to a capability checklist. + +| Status | Meaning | +| ------ | ------------------------------------------------------------------------------------------------------------------- | +| ✅ | Supported, **and a test proves it**. The test is linked; a doc page is not enough for a ✅. | +| ⚠️ | Supported, but nothing in this repo tests it end to end. Believe it less than a ✅. | +| ❌ | Not supported today. | +| ➖ | Not applicable — the sample is about app framework, deployment, or an AI SDK, not about what a contract layer does. | + +Tests live in `packages/*/src/__tests__/`. Anything named `*.inprocess.spec.ts` +runs against a real time-skipping Temporal server; `*.spec.ts` at the package +root is a unit test. + +## Core workflow and activity mechanics + +| Sample | Status | How, and what proves it | +| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [hello-world](https://github.com/temporalio/samples-typescript/tree/main/hello-world) | ✅ | `defineContract` + `declareWorkflow` + `declareActivitiesHandler`. `packages/worker/src/__tests__/one-call-fixture.inprocess.spec.ts` | +| [activities-examples](https://github.com/temporalio/samples-typescript/tree/main/activities-examples) | ✅ | Activities return `AsyncResult`; failures are `ApplicationFailure` via `qualifyFailure`. `packages/worker/src/activity.spec.ts` | +| [activities-dependency-injection](https://github.com/temporalio/samples-typescript/tree/main/activities-dependency-injection) | ✅ | `createContext` + activity middleware inject dependencies per invocation instead of module state. `packages/worker/src/__tests__/time-skipping.inprocess.spec.ts`, `packages/worker/src/activity-contract-errors.spec.ts` | +| [activities-cancellation-heartbeating](https://github.com/temporalio/samples-typescript/tree/main/activities-cancellation-heartbeating) | ✅ | Cancellation arrives as `ActivityCancelledError` on the modeled channel; `rethrowCancellation` re-raises it. `packages/worker/src/__tests__/cancellation.inprocess.spec.ts` | +| [timer-examples](https://github.com/temporalio/samples-typescript/tree/main/timer-examples) | ✅ | `sleep` / `condition` from `@temporalio/workflow` inside `implementation`. `packages/worker/src/__tests__/timeouts.inprocess.spec.ts` | +| [child-workflows](https://github.com/temporalio/samples-typescript/tree/main/child-workflows) | ✅ | `context.executeChildWorkflow` / `startChildWorkflow`, typed against the child's contract. `packages/worker/src/__tests__/child-wire.inprocess.spec.ts` | +| [continue-as-new](https://github.com/temporalio/samples-typescript/tree/main/continue-as-new) | ✅ | `context.continueAsNew`, with the run chain replayed in full. `packages/worker/src/__tests__/continue-as-new.inprocess.spec.ts` | +| [saga](https://github.com/temporalio/samples-typescript/tree/main/saga) | ✅ | `context.saga()` — LIFO undos, machinery failures exempt, undos in a non-cancellable scope. `packages/worker/src/__tests__/saga.inprocess.spec.ts` | +| [patching-api](https://github.com/temporalio/samples-typescript/tree/main/patching-api) | ⚠️ | `patched`/`deprecatePatch` from `@temporalio/workflow` work inside `implementation`; the contract layer neither helps nor hinders. No test here. | +| [worker-specific-task-queues](https://github.com/temporalio/samples-typescript/tree/main/worker-specific-task-queues) | ✅ | Per-activity `taskQueue` overrides. `packages/worker/src/__tests__/routing.spec.ts` | +| [mutex](https://github.com/temporalio/samples-typescript/tree/main/mutex) | ⚠️ | Expressible with signals + `condition` (the sample's own approach). No test here. | +| [batch-sliding-window](https://github.com/temporalio/samples-typescript/tree/main/batch-sliding-window) | ⚠️ | Expressible with child workflows + continue-as-new, both tested individually; the composition is not. | +| [dsl-interpreter](https://github.com/temporalio/samples-typescript/tree/main/dsl-interpreter) | ⚠️ | A DSL workflow takes its program as validated input like any other payload. No test here. | +| [expense](https://github.com/temporalio/samples-typescript/tree/main/expense) / [food-delivery](https://github.com/temporalio/samples-typescript/tree/main/food-delivery) | ✅ | Human-in-the-loop: signal-driven approval gate. `examples/order-processing-worker` (`processOrder`), `src/integration.spec.ts` | + +## Messages: signals, queries, updates + +| Sample | Status | How, and what proves it | +| ----------------------------------------------------------------------------------------------------- | ------ | --------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [signals-queries](https://github.com/temporalio/samples-typescript/tree/main/signals-queries) | ✅ | `defineSignal` / `defineQuery` + `context.handleSignal` / `handleQuery`, validated both sides. `packages/worker/src/__tests__/handlers.inprocess.spec.ts` | +| [message-passing](https://github.com/temporalio/samples-typescript/tree/main/message-passing) | ✅ | Updates too: `defineUpdate` + `context.handleUpdate`, with worker-side admission rejection typed as `UpdateRejectedError`. Same file. | +| [query-subscriptions](https://github.com/temporalio/samples-typescript/tree/main/query-subscriptions) | ⚠️ | Polling a typed query from the client works; the sample's streaming shape is app code. | +| [early-return](https://github.com/temporalio/samples-typescript/tree/main/early-return) | ⚠️ | Expressible with `startUpdate` + a later `result()`. No test here. | +| [state](https://github.com/temporalio/samples-typescript/tree/main/state) | ✅ | Workflow-local state read by a query. `packages/worker/src/__tests__/handlers.inprocess.spec.ts` | + +## Client, scheduling, and indexing + +| Sample | Status | How, and what proves it | +| ------------------------------------------------------------------------------------------------------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [schedules](https://github.com/temporalio/samples-typescript/tree/main/schedules) | ⚠️ | `client.schedule.create/getHandle/trigger`, typed against the contract, with `ScheduleAlreadyExistsError` modeled. Unit-tested against a stubbed client (`packages/client/src/schedule.spec.ts`); **no real-server test**. | +| [cron-workflows](https://github.com/temporalio/samples-typescript/tree/main/cron-workflows) | ➖ | Deprecated upstream in favour of schedules. | +| [search-attributes](https://github.com/temporalio/samples-typescript/tree/main/search-attributes) | ⚠️ | `defineSearchAttribute` + typed `searchAttributes` on start, `readTypedSearchAttributes` on read. Unit-tested (`packages/client/src/client.spec.ts`); **no real-server test** — and visibility is exactly what a real server tests differently. | +| [eager-workflow-start](https://github.com/temporalio/samples-typescript/tree/main/eager-workflow-start) | ⚠️ | A per-call Temporal option; passes through the typed start options untouched. | +| [standalone-activity](https://github.com/temporalio/samples-typescript/tree/main/standalone-activity) | ❌ | No client-side standalone activity execution. | +| [workflow-streams](https://github.com/temporalio/samples-typescript/tree/main/workflow-streams) | ⚠️ | Built on updates/queries, which are typed; the streaming wrapper is app code. | + +## Failure handling and retries + +| Sample | Status | How, and what proves it | +| ------------------------------------------------------------------------------------------- | ------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [polling](https://github.com/temporalio/samples-typescript/tree/main/polling) | ✅ | Retry policies per activity via `activityOptions` / `activityOptionsByName`. `packages/worker/src/__tests__/retry.inprocess.spec.ts` | +| [timer-progress](https://github.com/temporalio/samples-typescript/tree/main/timer-progress) | ✅ | Heartbeats and timeouts. `packages/worker/src/__tests__/timeouts.inprocess.spec.ts` | +| [sleep-for-days](https://github.com/temporalio/samples-typescript/tree/main/sleep-for-days) | ✅ | Long durable timers, time-skipped in tests. `packages/worker/src/__tests__/time-skipping.inprocess.spec.ts` | +| Typed domain errors (no upstream equivalent) | ✅ | `errors:` on an activity or workflow crosses the wire as `ApplicationFailure` and rehydrates as a typed `ContractError`. `packages/worker/src/__tests__/rehydration.inprocess.spec.ts` | + +## Nexus + +| Sample | Status | Notes | +| --------------------------------------------------------------------------------------------------------------------- | ------ | -------------------------------------------------------------------------- | +| [nexus-hello](https://github.com/temporalio/samples-typescript/tree/main/nexus-hello) | ❌ | No Nexus support, and no target release — see `docs/explanation/nexus.md`. | +| [nexus-cancellation](https://github.com/temporalio/samples-typescript/tree/main/nexus-cancellation) | ❌ | Same. | +| [nexus-messaging](https://github.com/temporalio/samples-typescript/tree/main/nexus-messaging) | ❌ | Same. | +| [nexus-standalone-operations](https://github.com/temporalio/samples-typescript/tree/main/nexus-standalone-operations) | ❌ | Same. | + +## Operations and worker configuration + +| Sample | Status | Notes | +| --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | -------------------------------------------------------------------------------------------------------------------------------------------- | +| [custom-logger](https://github.com/temporalio/samples-typescript/tree/main/custom-logger) | ⚠️ | Worker options pass through `TypedWorker.create`; `log.*` from `@temporalio/workflow` is the workflow-side path. | +| [sinks](https://github.com/temporalio/samples-typescript/tree/main/sinks) | ⚠️ | Passes through worker options untouched. No test here. | +| [interceptors-opentelemetry](https://github.com/temporalio/samples-typescript/tree/main/interceptors-opentelemetry) | ⚠️ | Temporal interceptors pass through; activity **middleware** is the contract-aware equivalent for the activity side. | +| [encryption](https://github.com/temporalio/samples-typescript/tree/main/encryption) / [protobufs](https://github.com/temporalio/samples-typescript/tree/main/protobufs) / [ejson](https://github.com/temporalio/samples-typescript/tree/main/ejson) | ⚠️ | A custom data converter is a worker/client option. Note the contract validates the **decoded** payload, so a converter and a schema compose. | +| [worker-versioning](https://github.com/temporalio/samples-typescript/tree/main/worker-versioning) | ⚠️ | Build IDs are worker options; nothing contract-specific. | +| [hello-world-mtls](https://github.com/temporalio/samples-typescript/tree/main/hello-world-mtls) / [env-config](https://github.com/temporalio/samples-typescript/tree/main/env-config) / [grpc-calls](https://github.com/temporalio/samples-typescript/tree/main/grpc-calls) | ➖ | Connection concerns — you build the `Client`/`NativeConnection`, we wrap it. | +| [production](https://github.com/temporalio/samples-typescript/tree/main/production) | ➖ | Deployment shape. | + +## Not applicable + +App-framework and AI-SDK samples, which say nothing about a contract layer: +[nestjs-exchange-rates](https://github.com/temporalio/samples-typescript/tree/main/nestjs-exchange-rates), +[nextjs-ecommerce-oneclick](https://github.com/temporalio/samples-typescript/tree/main/nextjs-ecommerce-oneclick), +[lambda-worker](https://github.com/temporalio/samples-typescript/tree/main/lambda-worker), +[monorepo-folders](https://github.com/temporalio/samples-typescript/tree/main/monorepo-folders), +[fetch-esm](https://github.com/temporalio/samples-typescript/tree/main/fetch-esm), +[vscode-debugger](https://github.com/temporalio/samples-typescript/tree/main/vscode-debugger), +[hello-world-js](https://github.com/temporalio/samples-typescript/tree/main/hello-world-js) (JavaScript, so no types to check), +and the agent samples +([ai-sdk](https://github.com/temporalio/samples-typescript/tree/main/ai-sdk), +[openai-agents](https://github.com/temporalio/samples-typescript/tree/main/openai-agents), +[google-adk-agents](https://github.com/temporalio/samples-typescript/tree/main/google-adk-agents), +[strands-agents](https://github.com/temporalio/samples-typescript/tree/main/strands-agents), +[langsmith](https://github.com/temporalio/samples-typescript/tree/main/langsmith)). + +## What this table says about the gaps + +- **Nexus is the one flat no.** Four samples, no support, no target release. +- **Schedules and search attributes are the weakest ✅-adjacent entries**: both + have a typed surface and unit tests against a stubbed client, and neither has + a real-server test — which is precisely the tier those two features need, + since visibility and schedule semantics are what a real cluster does + differently. +- Most ⚠️ rows are "Temporal's own API passes through untouched". That is + usually the right answer for a contract layer, but it is a claim this repo + does not currently test. diff --git a/README.md b/README.md index cfd378f5..c49be36c 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ End-to-end type safety and runtime validation for workflows and activities [![TypeScript](https://img.shields.io/badge/TypeScript-6.0-blue?logo=typescript)](https://www.typescriptlang.org/) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) -[**Documentation**](https://btravstack.github.io/temporal-contract) · [**Tutorial**](https://btravstack.github.io/temporal-contract/tutorial/your-first-workflow) · [**Reference**](https://btravstack.github.io/temporal-contract/reference/contract-surface) +[**Documentation**](https://btravstack.github.io/temporal-contract) · [**Tutorial**](https://btravstack.github.io/temporal-contract/tutorial/your-first-workflow) · [**Reference**](https://btravstack.github.io/temporal-contract/reference/contract-surface) · [**Sample coverage**](EXAMPLES.md) From 7f31cd3e76a7d13d379a8ea3d56c1fea24ba937d Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 09:54:13 +0200 Subject: [PATCH 09/12] fix: address review findings on the DX pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two were real defects, not nits: **The example's payment key could collide across orders.** It was `charge:${customerId}:${amount}`, so a customer placing two orders for the same amount would produce one key — and the mock gateway's ledger would "replay" the first charge, silently never charging the second order. The key now names the business operation (`charge:${orderId}`), which is why `orderId` joins `processPayment`'s input. The example taught the wrong shape, which is worse than not showing one. **The mock gateway only honoured the key for charges.** `refundPayment` declares one and Temporal retries refunds like anything else, so the adapter now keeps a refunded-key set too. **Renaming `idempotency` could silently downgrade a plain-JS contract.** With the field gone, a definition still carrying the old name reached the client with no policy at all and inherited Temporal's ALLOW_DUPLICATE. Contract validation now rejects it by name; carrying both fields is fine (only a missing `startPolicy` is fatal). Also: the time-skipping fixture now shuts down a worker left RUNNING, not just one left INITIALIZED; `qualifyFailure`'s required `expected` option is supplied in the changeset example and in the README's headline example (which did not compile, and predates this branch); the changeset no longer claims the pattern groups mirror a method's union "exactly" when declared contract errors are excluded; and the doc-link fragments, the `#declare-idempotency` anchor, the `IdempotencyMode` references, an incomplete example input, and a typo in the deprecation note all follow the rename. Not taken: `activity-failure.spec.ts:82` uses a bare `Error` on purpose — the test asserts that a NON-`ActivityError` value is rethrown unchanged, so a concrete error class would weaken it. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .changeset/activity-idempotency-key.md | 5 ++- .changeset/client-error-patterns.md | 6 ++- README.md | 11 ++--- docs/how-to/define-a-contract.md | 2 +- docs/how-to/test-workflows.md | 2 +- docs/reference/contract-surface.md | 6 +-- docs/reference/errors.md | 2 +- docs/reference/worker-surface.md | 2 +- docs/tutorial/your-first-workflow.md | 4 +- .../order-processing-contract/src/contract.ts | 10 +++-- .../src/application/workflows.ts | 6 ++- .../adapters/payment.adapter.ts | 12 ++++++ packages/contract/src/builder.spec.ts | 40 +++++++++++++++++++ packages/contract/src/builder.ts | 13 ++++++ packages/contract/src/idempotency.ts | 2 +- packages/contract/src/internal.ts | 2 +- packages/testing/src/time-skipping.ts | 26 +++++++----- 17 files changed, 119 insertions(+), 32 deletions(-) diff --git a/.changeset/activity-idempotency-key.md b/.changeset/activity-idempotency-key.md index 387cae0d..f2f150f7 100644 --- a/.changeset/activity-idempotency-key.md +++ b/.changeset/activity-idempotency-key.md @@ -14,7 +14,10 @@ const chargeCard = defineActivity({ }); chargeCard: ({ input, idempotencyKey }) => - fromPromise(gateway.charge(input, { idempotencyKey }), qualifyFailure("CHARGE_FAILED")), + fromPromise( + gateway.charge(input, { idempotencyKey }), + qualifyFailure("CHARGE_FAILED", { expected: GatewayError }), + ), ``` Temporal runs activities **at least once**, and nothing in the library helped diff --git a/.changeset/client-error-patterns.md b/.changeset/client-error-patterns.md index 0486daa4..148c0a7f 100644 --- a/.changeset/client-error-patterns.md +++ b/.changeset/client-error-patterns.md @@ -10,5 +10,7 @@ union exactly, so `matcher.with(...WORKFLOW_RESULT_PATTERNS, handler)` replaces six hand-written `P.tag(...)` arguments. Exhaustiveness is unchanged: these are ordinary pattern tuples, so a missing -member is still a compile error naming it. Contract errors are deliberately -excluded — match those first with `{ errorName: "..." }`. +member is still a compile error naming it. A workflow's **declared contract +errors** are deliberately not in these groups — no shipped group can name a +user's own errors — so for a workflow that declares `errors`, a group alone is +not exhaustive: match those first with `{ errorName: "..." }`. diff --git a/README.md b/README.md index c49be36c..f9201780 100644 --- a/README.md +++ b/README.md @@ -77,11 +77,12 @@ export const activities = declareActivitiesHandler({ activities: { processOrder: { chargeCard: ({ input: { customerId, amount } }) => - fromPromise(gateway.charge(customerId, amount), qualifyFailure("CHARGE_FAILED")).map( - (charge) => ({ - transactionId: charge.id, - }), - ), + fromPromise( + gateway.charge(customerId, amount), + // `expected` names the failures this activity anticipates; anything + // else rides the defect channel with its original stack. + qualifyFailure("CHARGE_FAILED", { expected: GatewayError }), + ).map((charge) => ({ transactionId: charge.id })), }, }, }); diff --git a/docs/how-to/define-a-contract.md b/docs/how-to/define-a-contract.md index d65d6ff9..89e07be6 100644 --- a/docs/how-to/define-a-contract.md +++ b/docs/how-to/define-a-contract.md @@ -36,7 +36,7 @@ reusable across workflows and contracts, give you precise hover and jump-to-definition, and keep the contract itself readable as a table of contents. -## Declare idempotency +## Declare a start policy `startPolicy` is required on every workflow. It answers one question: **is it safe to start this workflow ID again after a previous run has closed?** diff --git a/docs/how-to/test-workflows.md b/docs/how-to/test-workflows.md index d4fc769d..82867157 100644 --- a/docs/how-to/test-workflows.md +++ b/docs/how-to/test-workflows.md @@ -382,7 +382,7 @@ describe("order processing", () => { const result = await worker.raw.runUntil(async () => client.executeWorkflow("processOrder", { workflowId: `order-${Date.now()}`, - args: { orderId: "ORD-1" }, + args: { orderId: "ORD-1", customerId: "CUST-1", amount: 42 }, }), ); diff --git a/docs/reference/contract-surface.md b/docs/reference/contract-surface.md index cd0ecfd9..31c5680f 100644 --- a/docs/reference/contract-surface.md +++ b/docs/reference/contract-surface.md @@ -61,7 +61,7 @@ relaxes when the contract exists purely to serve activities. | ------------------ | ------------------------------------------- | -------- | | `input` | `AnySchema` | yes | | `output` | `AnySchema` | yes | -| `startPolicy` | `IdempotencyMode` | yes | +| `startPolicy` | `WorkflowStartPolicy` | yes | | `activities` | `Record` | no | | `signals` | `Record` | no | | `queries` | `Record` | no | @@ -84,7 +84,7 @@ workflow; an explicit per-call `workflowIdReusePolicy` overrides it. It is this mode; see [Schedule workflows](/how-to/schedule-workflows) for the implications. `workflowIdConflictPolicy` — what to do about a run that is already _open_ — stays a per-call client/worker option, untouched by this -field. See [Define a contract](/how-to/define-a-contract#declare-idempotency). +field. See [Define a contract](/how-to/define-a-contract#declare-a-start-policy). ### `defineActivity(definition)` @@ -271,7 +271,7 @@ See the [errors reference](/reference/errors). `AnySchema`, `UndefinedInputSchema`, `ActivityDefinition`, `SignalDefinition`, `QueryDefinition`, `UpdateDefinition`, `WorkflowDefinition`, `AnyWorkflowDefinition`, `ContractDefinition`, `SearchAttributeDefinition`, -`SearchAttributeKind`, `SearchAttributeKindToType`, `IdempotencyMode` +`SearchAttributeKind`, `SearchAttributeKindToType`, `WorkflowStartPolicy` `UndefinedInputSchema` is the Standard Schema type materialized by `defineSignal` / `defineQuery` / `defineUpdate` when `input` is omitted — diff --git a/docs/reference/errors.md b/docs/reference/errors.md index 0260f225..e436624d 100644 --- a/docs/reference/errors.md +++ b/docs/reference/errors.md @@ -304,7 +304,7 @@ every non-cancellation failure lands here. `originalFailure` exists so `propagateFailure` can re-raise the exact failure Temporal originally produced without changing what `cause` means for existing consumers that narrow on it — see [Worker -surface](/reference/worker-surface#propagateactivityfailure-result). +surface](/reference/worker-surface#propagatefailure-result). ### `ActivityCancelledError` diff --git a/docs/reference/worker-surface.md b/docs/reference/worker-surface.md index 05de6580..1577bd4e 100644 --- a/docs/reference/worker-surface.md +++ b/docs/reference/worker-surface.md @@ -350,7 +350,7 @@ step did before saying no is knowable. They do **not** run on an `ActivityError`, a `ChildWorkflowError` or a defect: a step that failed unmodelled left state nobody can see, and un-deciding what you cannot see is a second bug. That failure propagates untouched, so -[`propagateFailure`](#propagateactivityfailure-result) still re-raises +[`propagateFailure`](#propagatefailure-result) still re-raises Temporal's original failure. Cancellation is the one case a caller may opt back in to, with diff --git a/docs/tutorial/your-first-workflow.md b/docs/tutorial/your-first-workflow.md index 2cd76e84..99a9b3c0 100644 --- a/docs/tutorial/your-first-workflow.md +++ b/docs/tutorial/your-first-workflow.md @@ -141,8 +141,8 @@ Three things to notice: - `taskQueue` lives on the contract, so neither the worker nor the client has to repeat it. - `startPolicy` is required on every workflow — it is what stops a retried - start from re-running a workflow that already finished. See [Define a - contract](/how-to/define-a-contract#declare-idempotency) for the three + start from re-running a workflow that already completed successfully. See [Define a + contract](/how-to/define-a-contract#declare-a-start-policy) for the three modes and why the field exists. `defineContract` validates this structure the moment it runs. Misspell a key, diff --git a/examples/order-processing-contract/src/contract.ts b/examples/order-processing-contract/src/contract.ts index 19cd6f4a..79aa1f02 100644 --- a/examples/order-processing-contract/src/contract.ts +++ b/examples/order-processing-contract/src/contract.ts @@ -88,13 +88,17 @@ const purgeExpiredOrders = defineActivity({ * a typed `ContractError` on the activity call's error channel. */ const processPayment = defineActivity({ - input: z.object({ customerId: z.string(), amount: z.number() }), + // `orderId` is here for the idempotency key rather than for the charge + // itself: the key has to name the *business operation*, and a customer may + // legitimately place two orders for the same amount. + input: z.object({ orderId: z.string(), customerId: z.string(), amount: z.number() }), output: PaymentResultSchema, // Temporal runs an activity AT LEAST once — a retry, a worker crash, or a // completion that succeeded but was never recorded all re-run this. The // key travels to the gateway so the second run settles the first charge - // instead of making a new one. - idempotencyKey: ({ customerId, amount }) => `charge:${customerId}:${amount}`, + // instead of making a new one. Keying on customer + amount would collide + // across two distinct orders and swallow the second charge entirely. + idempotencyKey: ({ orderId }) => `charge:${orderId}`, errors: { PaymentDeclined: paymentDeclinedError, }, diff --git a/examples/order-processing-worker/src/application/workflows.ts b/examples/order-processing-worker/src/application/workflows.ts index 8f6a5a06..226e032a 100644 --- a/examples/order-processing-worker/src/application/workflows.ts +++ b/examples/order-processing-worker/src/application/workflows.ts @@ -154,7 +154,11 @@ export const processOrder = declareWorkflow({ // channels once, at the call site — every arm either produces a value or // deliberately ends the workflow. const paymentOutcome = await activities - .processPayment({ customerId: order.customerId, amount: order.totalAmount }) + .processPayment({ + orderId: order.orderId, + customerId: order.customerId, + amount: order.totalAmount, + }) .match({ ok: (payment) => ({ kind: "paid" as const, payment }), errCases: (matcher) => diff --git a/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts b/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts index c4b0665b..a57fff9f 100644 --- a/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts +++ b/examples/order-processing-worker/src/infrastructure/adapters/payment.adapter.ts @@ -16,6 +16,9 @@ export class MockPaymentAdapter implements PaymentPort { */ private readonly settled = new Map(); + /** Refunds already issued, by idempotency key — same contract, same reason. */ + private readonly refunded = new Set(); + async processPayment( customerId: string, amount: number, @@ -73,6 +76,14 @@ export class MockPaymentAdapter implements PaymentPort { } async refundPayment(transactionId: string, idempotencyKey: string): Promise { + if (this.refunded.has(idempotencyKey)) { + // A refund is as unsafe to repeat as a charge: `refundPayment` is + // retried by Temporal like any other activity, and this workflow also + // runs it as a compensation. + logger.info({ idempotencyKey }, `↩️ Refund already issued for ${idempotencyKey}`); + return; + } + logger.info( { transactionId, idempotencyKey }, `💰 Processing refund for transaction ${transactionId}`, @@ -83,6 +94,7 @@ export class MockPaymentAdapter implements PaymentPort { if (success) { logger.info(`✅ Refund successful`); + this.refunded.add(idempotencyKey); } else { logger.error(`❌ Refund failed`); // oxlint-disable-next-line unthrown/no-throw -- known-technical precondition throw in a plain (non-Result) domain helper, wrapped once at the activity boundary via fromPromise(..., qualifyFailure(...)) diff --git a/packages/contract/src/builder.spec.ts b/packages/contract/src/builder.spec.ts index fe00c247..1ae8743b 100644 --- a/packages/contract/src/builder.spec.ts +++ b/packages/contract/src/builder.spec.ts @@ -946,6 +946,46 @@ describe("Contract Builder", () => { ); }); + it("should reject the pre-rename `idempotency` field instead of silently ignoring it", () => { + // A plain-JS contract or a stale compiled artifact can still carry the + // old field. Ignoring it would leave the workflow with no policy at + // all — Temporal's ALLOW_DUPLICATE — silently dropping the protection + // the author thought they had declared. + expect(() => + defineContract({ + taskQueue: "test", + workflows: { + test: { + input: z.object({}), + output: z.object({}), + // @ts-expect-error - the pre-rename field, as untyped callers still have it + idempotency: "once-per-id", + }, + }, + }), + ).toThrow('"idempotency" was renamed to "startPolicy"'); + }); + + it("accepts a definition carrying BOTH fields, taking the new one", () => { + // Mid-migration codebases exist; only a *missing* startPolicy is fatal. + expect(() => + defineContract({ + taskQueue: "test", + workflows: { + test: { + input: z.object({}), + output: z.object({}), + startPolicy: "once-per-id", + // Not a type error: the definition generic infers the literal, + // so a leftover key rides along. That is exactly why the + // runtime check above has to exist. + idempotency: "allow-duplicate", + }, + }, + }), + ).not.toThrow(); + }); + it("should throw when workflow startPolicy is not a string", () => { expect(() => defineContract({ diff --git a/packages/contract/src/builder.ts b/packages/contract/src/builder.ts index 012781ea..c1672ab8 100644 --- a/packages/contract/src/builder.ts +++ b/packages/contract/src/builder.ts @@ -768,6 +768,19 @@ function validateWorkflowDefinition(context: string, definition: unknown): void // client's/worker's own `definition.startPolicy ? {...} : {}` guards stay // defensive for exactly that case, and the `plainWorkflow` fixture in // client.spec.ts exists to prove it. + // A definition still carrying the pre-rename `idempotency` field fails + // loudly rather than silently losing its policy. The type system catches + // this for TypeScript callers, but a plain-JS contract or a stale compiled + // artifact would otherwise reach the client with no `startPolicy` at all + // and inherit Temporal's `ALLOW_DUPLICATE` — silently dropping the very + // protection the field exists to declare. + if (definition["idempotency"] !== undefined && startPolicy === undefined) { + fail( + `${context}: "idempotency" was renamed to "startPolicy". Rename the field ` + + `(the mode values are unchanged) — leaving it as "idempotency" would ` + + `silently fall back to Temporal's ALLOW_DUPLICATE.`, + ); + } if ( startPolicy !== undefined && startPolicy !== "once-per-id" && diff --git a/packages/contract/src/idempotency.ts b/packages/contract/src/idempotency.ts index ffea5a1d..170730c9 100644 --- a/packages/contract/src/idempotency.ts +++ b/packages/contract/src/idempotency.ts @@ -50,7 +50,7 @@ export function reusePolicyFor(mode: WorkflowStartPolicy): WorkflowIdReusePolicy /** * @deprecated Renamed to {@link WorkflowStartPolicy}, and the field that - * carries it from `startPolicy` to `startPolicy`: it governs + * carries it from `idempotency` to `startPolicy`: it governs * `workflowIdReusePolicy` — whether a workflow ID may be reused after a * Closed run — and never made a workflow idempotent. For an activity running * twice under Temporal's at-least-once guarantee, see an activity's diff --git a/packages/contract/src/internal.ts b/packages/contract/src/internal.ts index f38643ef..7d684e4d 100644 --- a/packages/contract/src/internal.ts +++ b/packages/contract/src/internal.ts @@ -27,7 +27,7 @@ export { * API: contract authors only ever set `startPolicy` on `defineWorkflow`; the * client and worker are the ones that translate it to Temporal's * `workflowIdReusePolicy` via this function, so it lives here rather than on - * `.` alongside the public `IdempotencyMode` type. + * `.` alongside the public `WorkflowStartPolicy` type. */ export { reusePolicyFor as _internal_reusePolicyFor } from "./idempotency.js"; diff --git a/packages/testing/src/time-skipping.ts b/packages/testing/src/time-skipping.ts index 4747a604..55ad9caf 100644 --- a/packages/testing/src/time-skipping.ts +++ b/packages/testing/src/time-skipping.ts @@ -210,16 +210,24 @@ export function createTimeSkippingContractTest { From fe23061817e312660e60f97b0094a4db27a9485f Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 10:55:18 +0200 Subject: [PATCH 10/12] refactor(worker)!: drop the propagateActivityFailure alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Knip failed the build on it — a duplicate export, which is exactly what a same-value alias is. Removing it rather than adding a knip exception, because keeping it was already incoherent with this branch: the same PR renames `idempotency` to `startPolicy` with no alias at all. Both names only ever shipped in 8.0 betas, so being gentle about one and ruthless about the other bought nothing. The upgrade guide now documents the rename with a diff, alongside `bestEffort`. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .../best-effort-and-propagate-rename.md | 5 +++-- docs/how-to/upgrade-to-v8.md | 21 +++++++++++++++++++ packages/worker/src/activity-failure.spec.ts | 10 +-------- packages/worker/src/activity-failure.ts | 8 ------- packages/worker/src/workflow.ts | 5 +---- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/.changeset/best-effort-and-propagate-rename.md b/.changeset/best-effort-and-propagate-rename.md index 88b4e4f9..67ee57d8 100644 --- a/.changeset/best-effort-and-propagate-rename.md +++ b/.changeset/best-effort-and-propagate-rename.md @@ -12,5 +12,6 @@ hand-written best-effort fold; it is now structural. `propagateActivityFailure` is renamed to **`propagateFailure`** — it has always also handled child-workflow calls and cancellation scopes, and the old name said -otherwise. The old name stays as a deprecated alias (the identical function -reference) and will be removed in the next major. +otherwise. The old name is **removed**, not aliased: it only ever shipped in 8.0 +betas, and this release already renames `idempotency` to `startPolicy` outright. +Rename the import; behaviour is unchanged. diff --git a/docs/how-to/upgrade-to-v8.md b/docs/how-to/upgrade-to-v8.md index 86850591..df90425f 100644 --- a/docs/how-to/upgrade-to-v8.md +++ b/docs/how-to/upgrade-to-v8.md @@ -939,6 +939,27 @@ worker-initiated child-workflow starts each have a dedicated integration suite that starts real executions and checks which ones the server actually accepts or rejects. +### `propagateActivityFailure` is now `propagateFailure` + +Same function, honest name: it has always also handled child-workflow calls +(`ChildWorkflowError`, `ChildWorkflowCancelledError`) and cancellation scopes +(`WorkflowCancelledError`), not just activity calls. + +```diff +-import { propagateActivityFailure } from "@temporal-contract/worker/workflow"; +-await propagateActivityFailure(context.activities.sendEmail(input)); ++import { propagateFailure } from "@temporal-contract/worker/workflow"; ++await propagateFailure(context.activities.sendEmail(input)); +``` + +No alias is kept: the old name only ever shipped in 8.0 betas. Behaviour is +unchanged, so the rename is the whole migration. + +Its new counterpart, `bestEffort(result, onFailure)`, covers the other half — +a call whose failure is worth a warning rather than the workflow. It re-raises +real cancellation, which a hand-written best-effort fold has to remember not to +absorb. + ### Let the contract derive the workflow ID `startPolicy` only bites if two starts of the same logical request actually diff --git a/packages/worker/src/activity-failure.spec.ts b/packages/worker/src/activity-failure.spec.ts index 8b59394f..ad0ea492 100644 --- a/packages/worker/src/activity-failure.spec.ts +++ b/packages/worker/src/activity-failure.spec.ts @@ -3,7 +3,7 @@ import { ApplicationFailure, ActivityFailure, RetryState } from "@temporalio/com import { ErrAsync, OkAsync } from "unthrown"; import { describe, expect, it } from "vitest"; -import { bestEffort, propagateActivityFailure, propagateFailure } from "./activity-failure.js"; +import { bestEffort, propagateFailure } from "./activity-failure.js"; import { ActivityCancelledError, ActivityError, @@ -201,14 +201,6 @@ describe("propagateFailure", () => { }); }); -describe("propagateActivityFailure (deprecated alias)", () => { - it("is the same function as propagateFailure", () => { - // Not a behavioural copy — the identical reference, so the alias cannot - // drift from the helper it stands in for. - expect(propagateActivityFailure).toBe(propagateFailure); - }); -}); - describe("bestEffort", () => { it("returns the value and never calls onFailure on Ok", async () => { const seen: unknown[] = []; diff --git a/packages/worker/src/activity-failure.ts b/packages/worker/src/activity-failure.ts index 9362f02e..849ff873 100644 --- a/packages/worker/src/activity-failure.ts +++ b/packages/worker/src/activity-failure.ts @@ -162,14 +162,6 @@ export async function propagateFailure(result: AsyncResult): Promise throw error; } -/** - * @deprecated Renamed to {@link propagateFailure}: this helper has always - * handled child-workflow calls and cancellation scopes too, not just activity - * calls, and the old name said otherwise. Behaviourally identical; it will be - * removed in the next major. - */ -export const propagateActivityFailure = propagateFailure; - /** * Await a call whose failure is **not** worth ending the workflow over — a * notification, a metric, an audit write — and hand that failure to diff --git a/packages/worker/src/workflow.ts b/packages/worker/src/workflow.ts index 3262f959..ffaa300c 100644 --- a/packages/worker/src/workflow.ts +++ b/packages/worker/src/workflow.ts @@ -118,10 +118,7 @@ export { rethrowCancellation } from "./errors.js"; // - `bestEffort` — "log it and carry on", for a non-critical call. Real // cancellation is still re-raised, so absorbing a cancel is not something // each call site has to remember. -// -// `propagateActivityFailure` is the deprecated former name of -// `propagateFailure`. -export { bestEffort, propagateActivityFailure, propagateFailure } from "./activity-failure.js"; +export { bestEffort, propagateFailure } from "./activity-failure.js"; // The saga, reachable without a context for the workflow that composes its // steps in a helper. `context.saga` is this same function. From ff70a65cb00e746a00e0974dddf974781be94958 Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 12:35:29 +0200 Subject: [PATCH 11/12] docs: key the idempotency examples on identity, not on parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: `${customerId}:${amount}` was a bad illustration. It describes a charge rather than identifying one — the same customer placing two orders of the same value collides on a single key, and a gateway swallows the second charge as a replay of the first. The example contract was already fixed for this; the canonical illustrations were not. `defineActivity`'s JSDoc, the changeset, and the spec fixtures now key on `charge:${orderId}`, with `orderId` added to the input for the key's sake. The JSDoc also names the three good sources, since "what makes a good key" is the part that is easy to get wrong: a business identifier already in the input; a dedicated `idempotencyKey` field the caller mints when no natural one exists; or the workflow ID, which is per-execution and — now that a contract can derive it from the payload — is itself a function of the input, readable in an activity via `Context.current().info.workflowExecution.workflowId`. Adds the regression the feedback describes: two orders of the same value must not produce the same key. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- .changeset/activity-idempotency-key.md | 10 ++- packages/contract/src/types.ts | 27 +++++++- .../worker/src/activity-idempotency.spec.ts | 63 ++++++++++++++----- 3 files changed, 81 insertions(+), 19 deletions(-) diff --git a/.changeset/activity-idempotency-key.md b/.changeset/activity-idempotency-key.md index f2f150f7..713d4e07 100644 --- a/.changeset/activity-idempotency-key.md +++ b/.changeset/activity-idempotency-key.md @@ -8,9 +8,11 @@ Activities can declare an **idempotency key**, derived from their input: ```ts const chargeCard = defineActivity({ - input: z.object({ customerId: z.string(), amount: z.number() }), + input: z.object({ orderId: z.string(), customerId: z.string(), amount: z.number() }), output: PaymentSchema, - idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, + // Key on what IDENTIFIES the charge, not on what describes it: one customer + // placing two orders of the same value must not collide on one key. + idempotencyKey: ({ orderId }) => `charge:${orderId}`, }); chargeCard: ({ input, idempotencyKey }) => @@ -29,3 +31,7 @@ with the same input. `helpers.idempotencyKey` is typed `string` for an activity that declares one and `undefined` for one that does not, so reaching for a key that was never declared is a compile error. `runActivity` hands over the same value. + +Good key sources: a business identifier already in the input, a dedicated +`idempotencyKey` field the caller mints, or the workflow ID — which is +per-execution and, when the contract derives it, a function of the payload. diff --git a/packages/contract/src/types.ts b/packages/contract/src/types.ts index 60729462..a2b6b53e 100644 --- a/packages/contract/src/types.ts +++ b/packages/contract/src/types.ts @@ -168,12 +168,35 @@ export type ActivityDefinition< * (`` `charge:${orderId}` `` vs `` `refund:${orderId}` ``) — handing a * gateway one key for two opposite operations is the failure to avoid. * + * Key on the **identity of the operation**, not on its parameters. A + * customer and an amount describe a charge but do not identify it: the same + * customer legitimately placing two orders of the same value would produce + * one key, and the second charge would be swallowed as a replay of the + * first. Good sources, in rough order of preference: + * + * - a business identifier already in the input (`orderId`, `invoiceId`) — + * add it to the input schema if it is not there yet, as this example does; + * - a dedicated `idempotencyKey` field in the input, minted by the caller + * when no natural identifier exists; + * - the **workflow ID**, which is per-execution and — when the contract + * derives it (see {@link WorkflowDefinition.workflowId}) — is itself a + * function of the payload. Read it inside the activity from + * `Context.current().info.workflowExecution.workflowId`, and combine it + * with a per-call discriminator if the same activity runs more than once + * in a workflow. + * * @example * ```ts * const chargeCard = defineActivity({ - * input: z.object({ customerId: z.string(), amount: z.number() }), + * // `orderId` is in the input for the key's sake: it identifies the + * // charge, where customer and amount only describe it. + * input: z.object({ + * orderId: z.string(), + * customerId: z.string(), + * amount: z.number(), + * }), * output: PaymentSchema, - * idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, + * idempotencyKey: ({ orderId }) => `charge:${orderId}`, * }); * ``` */ diff --git a/packages/worker/src/activity-idempotency.spec.ts b/packages/worker/src/activity-idempotency.spec.ts index 6f22ddc8..e071f679 100644 --- a/packages/worker/src/activity-idempotency.spec.ts +++ b/packages/worker/src/activity-idempotency.spec.ts @@ -28,11 +28,13 @@ const contract = { }, }, activities: { - // Declares a key: the customer + amount pair a gateway must not charge twice. + // Declares a key on what IDENTIFIES the charge. Keying on customer + + // amount would describe it instead, and collide across two distinct + // orders of the same value. charge: { - input: z.object({ customerId: z.string(), amount: z.number() }), + input: z.object({ orderId: z.string(), customerId: z.string(), amount: z.number() }), output: z.object({ key: z.string() }), - idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, + idempotencyKey: ({ orderId }) => `charge:${orderId}`, }, // Declares none: reading a balance twice is harmless. readBalance: { @@ -63,9 +65,9 @@ describe("activity idempotency key", () => { }, }); - await expect(activities.charge({ customerId: "CUST-1", amount: 149.97 })).resolves.toEqual({ - key: "CUST-1:149.97", - }); + await expect( + activities.charge({ orderId: "ORD-1", customerId: "CUST-1", amount: 149.97 }), + ).resolves.toEqual({ key: "charge:ORD-1" }); }); it("produces the SAME key on a re-run of the same input", async () => { @@ -75,12 +77,43 @@ describe("activity idempotency key", () => { activities: { charge: echoKey, readBalance: echoKey, chargeTrimmed: echoKey }, }); - const first = await activities.charge({ customerId: "CUST-1", amount: 149.97 }); - const second = await activities.charge({ customerId: "CUST-1", amount: 149.97 }); + const first = await activities.charge({ + orderId: "ORD-1", + customerId: "CUST-1", + amount: 149.97, + }); + const second = await activities.charge({ + orderId: "ORD-1", + customerId: "CUST-1", + amount: 149.97, + }); expect(first).toEqual(second); }); + it("does NOT collide across two orders of the same value", async () => { + // The failure this key shape exists to avoid: keyed on customer + amount, + // these two legitimate orders would share a key and a gateway would + // swallow the second charge as a replay of the first. + const activities = declareActivitiesHandler({ + contract, + activities: { charge: echoKey, readBalance: echoKey, chargeTrimmed: echoKey }, + }); + + const first = await activities.charge({ + orderId: "ORD-1", + customerId: "CUST-1", + amount: 149.97, + }); + const second = await activities.charge({ + orderId: "ORD-2", + customerId: "CUST-1", + amount: 149.97, + }); + + expect(first).not.toEqual(second); + }); + it("hands over undefined when the activity declares no key", async () => { const activities = declareActivitiesHandler({ contract, @@ -109,8 +142,8 @@ describe("activity idempotency key", () => { // Middleware may replace the input (re-validated at the boundary); the // key must describe what actually ran, not what the caller sent. const rewrite = declareActivityMiddleware(({ input }, next) => { - const typed = input as { customerId: string; amount: number }; - return next({ input: { ...typed, customerId: "CUST-REWRITTEN" } }); + const typed = input as { orderId: string; customerId: string; amount: number }; + return next({ input: { ...typed, orderId: "ORD-REWRITTEN" } }); }); const activities = declareActivitiesHandler({ @@ -119,9 +152,9 @@ describe("activity idempotency key", () => { activities: { charge: echoKey, readBalance: echoKey, chargeTrimmed: echoKey }, }); - await expect(activities.charge({ customerId: "CUST-1", amount: 10 })).resolves.toEqual({ - key: "CUST-REWRITTEN:10", - }); + await expect( + activities.charge({ orderId: "ORD-1", customerId: "CUST-1", amount: 10 }), + ).resolves.toEqual({ key: "charge:ORD-REWRITTEN" }); }); }); @@ -130,9 +163,9 @@ describe("activity idempotency key — types", () => { // derivation's parameter is contextually typed: no annotation needed, and a // field the input doesn't have is a compile error. const charge = defineActivity({ - input: z.object({ customerId: z.string(), amount: z.number() }), + input: z.object({ orderId: z.string(), customerId: z.string(), amount: z.number() }), output: z.object({ ok: z.boolean() }), - idempotencyKey: ({ customerId, amount }) => `${customerId}:${amount}`, + idempotencyKey: ({ orderId }) => `charge:${orderId}`, }); const readBalance = defineActivity({ From ed7a2a727a75f6707e099da685de20bc971df69e Mon Sep 17 00:00:00 2001 From: Benoit TRAVERS Date: Thu, 3 Sep 2026 13:03:19 +0200 Subject: [PATCH 12/12] fix(testing): await the shutdown before the environment teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on the time-skipping fixture: `shutdown()` only *starts* the stop, so returning straight after it races the worker-scoped `testEnv` teardown that closes the native connection. Not the proposed `await rig.worker.raw.run()` — `run()` is what you await if you started the worker, and this fixture never does (the test does, via `runUntil` or its own `run()`), so there is no such promise to own here. `createContractTest` can await its `running` handle precisely because it started the worker itself. Waiting for the state to reach `STOPPED` is the equivalent without that handle. Claude-Session: https://claude.ai/code/session_01TV7EynACZGKKWJxsP6KLLY --- packages/testing/src/time-skipping.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/testing/src/time-skipping.ts b/packages/testing/src/time-skipping.ts index 55ad9caf..1d9b5d4d 100644 --- a/packages/testing/src/time-skipping.ts +++ b/packages/testing/src/time-skipping.ts @@ -49,7 +49,7 @@ import { type TimeSkippingTestWorkflowEnvironmentOptions, } from "@temporalio/testing"; import type { WorkflowBundleWithSourceMap } from "@temporalio/worker"; -import { it as vitestIt } from "vitest"; +import { it as vitestIt, vi } from "vitest"; import { testRig } from "./test-rig.js"; import { bundleFor } from "./workflow-bundle.js"; @@ -227,7 +227,13 @@ export function createTimeSkippingContractTest rig.worker.raw.getState() === "STOPPED", { interval: 100 }); } }, worker: async ({ rig }, use) => {