feat(worker)!: return AsyncResult from every activity call by btravers · Pull Request #370 · btravstack/temporal-contract

btravers · 2026-08-04T07:41:21Z

Workstream 4, part 1 of 3, of the production-hardening effort.

Previously the workflow-side activity call convention depended on a declaration in a different file: an activity with a declared errors map returned AsyncResult, one without returned a Promise that threw. The call site gave no signal which applied, and the throwing branch contradicted the library's own rule 2 — "Activities and the typed client return AsyncResult<T, E>. Never throw."

Now every activity call returns AsyncResult. The error channel still varies with the contract; the call convention no longer does.

The invariant this rests on

packages/worker/src/__tests__/propagation.inprocess.spec.ts is a characterization test written against the pre-change library, in the first commit, before any production code moved. It has been byte-identical ever since — verified by blob hash at all 13 commits — and still passes. That is the evidence this refactor preserved Temporal's failure semantics rather than merely compiling.

It asserts failed:WorkflowFailedError:ActivityFailure:activity exploded, which discriminates a real workflow failure from a workflow-task retry loop. That distinction is the whole game: ActivityError is a TaggedError, not a TemporalFailure, and Temporal turns a thrown non-TemporalFailure into a task retry loop that stalls the workflow until its execution timeout instead of failing it. The oracle's discrimination was itself proven by injecting the regression and observing WorkflowTimeoutError.

API

// Handle it:
const result = await context.activities.charge(input);
if (result.isErr()) { /* ... */ }

// Or let Temporal decide the workflow's fate:
import { propagateActivityFailure } from "@temporal-contract/worker/workflow";
await propagateActivityFailure(context.activities.charge(input));

Do not use unthrown's .getOrThrow() for this. It throws the ActivityError wrapper — not a TemporalFailure — producing the stall described above. propagateActivityFailure re-raises the preserved original failure, so Temporal sees exactly what it saw before.

Breaking changes

Every activity call returns AsyncResult; calls to activities without declared errors must narrow or propagate.
ActivityError gains originalFailure, holding the pre-unwrap ActivityFailure. cause keeps its existing meaning.
cancellableScope(() => ctx.activities.x(...)) now yields a nested AsyncResult and stops compiling — AsyncResult is deliberately not a PromiseLike. Await and narrow inside the callback.
Cancellation of an activity without declared errors now rides Err(ActivityCancelledError) rather than escaping. Swallowing it completes a workflow as Completed instead of Cancelled; that hazard previously applied only to errors-declaring activities.

The hazard that is not compiler-caught

await context.activities.sendEmail(input); compiles identically before and after, and now silently swallows the failure. Seven real instances were found during this work — three in worker fixtures that tsc never flagged, four in the examples, including one where cancellableScope double-wrapped a Result so ok: () => undefined discarded the inner failure and the cancellation, in code whose own comment asserted cancellation must propagate.

The repo's unthrown/no-unhandled-result lint rule does not fire on the proxy call, so grep is the only mechanical remedy. The upgrade guide and changeset now say so, with the exact pattern to search for.

Review

Six tasks, each with an independent reviewer, plus a whole-branch review and scoped re-reviews of every fix round.

Findings that mattered, nearly all originating in the plan rather than the implementation:

The regression oracle initially could not detect its own regression. result.isErr() is true both when Temporal fails the workflow and when a task-retry loop times out. The 10s timeout added as a hang-mitigation is what turned a visible 120s stall into a fast false green.
The first propagation implementation threw the wrong failure type — classifyActivityError unwraps and discards the ActivityFailure wrapper, so rethrowing cause would have silently changed what clients see.
The fix then stalled the majority case: declared-error activities produce a ContractError, also a TaggedError, which fell through to a bare rethrow.
Thirteen stale documentation statements, several in files an earlier round had already edited — including the published npm README and .agents/rules/handlers.md, which is self-propagating since CLAUDE.md points at it.
Four rounds shipped a non-compiling doc example, each blessed by reading rather than compiling. The later reviews built probes outside the repo and proved them sensitive — reproducing a known-bad shape first — before trusting any green result.

Two commits were authored by the coordinating session rather than an implementer, after a quota limit and an infrastructure stall; both were verified item-by-item and are flagged as such.

Verification

Uncached pnpm turbo run typecheck 12/12 · pnpm turbo run test 9/9 · in-process tier 61/61 · Docker integration tier green · oxlint clean.

Note turbo run test runs only the unit project — the in-process tier that holds the oracle must be run explicitly.

Follow-ups (pre-existing, none introduced here)

workflow.ts:205-207 — the flagship declareWorkflow JSDoc folds an activity Err into the same string the business outcome returns, mapping a technical fault to a business decline and absorbing cancellation.
upgrade-to-v8.md:526 — a surviving discarded await.
examples/order-processing-worker/.../workflows.ts:150-152 — states the deleted rule verbatim.
troubleshoot.md:315-325 — prescribes a catch/isCancellation that can no longer fire.
the-result-model.md:245-248 — claims schedule handle methods return AsyncResult<void, never>; they declare ScheduleNotFoundError.
nexus.md:90 — reads process.env inside a declareWorkflow implementation, one of the three genuinely unprotected determinism hazards in rule 1.
ChildWorkflowError.cause needs the same originalFailure retention for a future propagateChildWorkflowFailure.

…sult change

…on oracle Round 1 fix: `isErr()` alone couldn't tell a correctly-failed workflow (WorkflowFailedError) apart from a workflow-task retry loop running out the execution timeout (WorkflowTimeoutError) — both satisfy isErr(), so the oracle was a false green against the exact regression it exists to catch. Fold the error's identity and inner cause into the assertion, and fold the handled-path's caught-error identity plus attempt count too, so neither test can pass without proving what actually happened.

…aising classifyActivityError unwraps Temporal's ActivityFailure into ActivityError.cause, discarding the wrapper. Rethrowing cause alone would hand Temporal a bare ApplicationFailure instead of the original ActivityFailure, changing the client-visible WorkflowFailedError.cause type. Retain the pre-unwrap value as ActivityError.originalFailure (cause's meaning is unchanged) and have propagateActivityFailure re-raise it, so a workflow awaiting an errors-declaring activity's AsyncResult can let a failure escape with identical Temporal semantics to a native throw.

…etention propagateActivityFailure's instanceof checks missed ContractError — the rehydrated shape for an activity's declared errors — which is a TaggedError, not a TemporalFailure, so it fell through to a bare rethrow and reproduced the workflow-task stall this helper exists to prevent. Add a ContractError branch that re-raises its cause (always the original ApplicationFailure). Also close a coverage gap: no existing test threw a real ActivityFailure wrapper through the proxy, so classifyActivityError's unwrap branch and the new originalFailure retention had zero assertions on it. Add two activities-proxy.spec.ts cases driving a genuine ActivityFailure (plain and cancellation-wrapped) through the real proxy in the unit project.

…ilure Round-1 review found two false claims in the ContractError doc block: (1) it claimed the same stall risk as ActivityError/ActivityCancelledError, but declareWorkflow's own catch (workflow.ts:411-419) recognizes and converts ContractError, so it never stalls — it misclassifies instead, looking the error name up on the workflow's errors map instead of the activity's, producing a wrong "not declared on workflow" failure; (2) it claimed ContractError never had a wrapper to preserve, but classifyActivityError does hold the original ActivityFailure wrapper at that point (it's passed to ActivityError as originalFailure) — it's just never threaded through the rehydration path into ContractError. Comments only; no behavior change.

…ntion Task 3 made every workflow-side activity call return AsyncResult regardless of whether the contract declares errors. Migrate the four in-package fixtures that still assumed the old shape: - routing.workflows.ts, timeouts.workflows.ts: success-only fixtures now unwrap via propagateActivityFailure instead of a bare await. - test.workflows.ts: workflowWithActivities unwraps technical activity failures with propagateActivityFailure while still branching on the Ok value's business fields (valid/success); childWorkflow's logMessage call and workflowWithFailableActivity (whose skipped spec expects the workflow itself to fail) now do the same. - activities-proxy.spec.ts: the two unit tests asserting a bare resolved/rejected value now assert the Result shape instead.

Migrate the order-processing-worker example's 8 activity call sites to the AsyncResult convention every activity call now returns. Choices are deliberate, not mechanical: narrow with .match() where a meaningful recovery path exists (notifications stay best-effort, a failed inventory reservation folds into the existing rollback), and use propagateActivityFailure only where letting Temporal fail the workflow is genuinely the intent (a failed refund after a failed reservation, a failed shipment with no compensating action, a failed scheduled purge with no recovery path). Also fixes 4 call sites that silently swallowed activity failures and were invisible to tsc: three discarded `await activities.x(...)` calls, plus a subtler case where `sendNotification` was wrapped in `cancellableScope`, which pre-uniform-AsyncResult correctly folded a thrown cancellation into a Result but now nests the activity's own Result inside cancellableScope's Result, so the outer `ok: () => undefined` handler was silently discarding the inner Err. Removed the now-redundant cancellableScope wrapper (a single activity call's own AsyncResult already carries ActivityCancelledError) and narrowed directly, matching the idiom already used elsewhere in the file.

… of stock Round-1 review fixes for the activity-Result migration: - reserveInventory's technical-failure arm folded into the same { reserved: false } shape the business "no stock" outcome uses, so a retries-exhausted/timeout failure was reported to the client as OUT_OF_STOCK. Carry an `unavailable` flag through both match arms and use it to select a distinct INVENTORY_UNAVAILABLE errorCode / "Inventory could not be reserved" failureReason and notification wording, mirroring processPayment's existing PAYMENT_UNAVAILABLE vs. PaymentDeclined split. The shared refund+notify rollback path is unchanged. - Document why the three notification defect arms warn-and-continue while the two business-outcome folds rethrow the defect at the edge: a notification defect is still just a failed email, and the order outcome already being returned/rethrown is authoritative. - Note in refundPayment's comment that propagateActivityFailure there restores the exact pre-uniform-AsyncResult behavior (an unhandled failure used to throw and fail the workflow outright).

…vention

…surface gaps Fix round 1 of 5 on the uniform-activity-result docs pass: AsyncResult is not a full PromiseLike, so cancellableScope/nonCancellableScope callbacks must await and narrow an activity's AsyncResult inside the callback, not outside it. Also fills gaps worker-surface.md and the changeset's migration notes missed.

…y claims

Fix round 3 of 5: errors.ts's rethrowCancellation example still taught the un-awaited-scope-callback and discarded-cleanup-Result shapes fixed everywhere else (blocking — published API reference, cross-referenced from worker-surface.md). Also standardizes releaseResources cleanup guidance on the quiet/best-effort policy across cancellation.ts, workflow.ts, and workflow-determinism.md, closes a missing outer-scope defect check, and qualifies an upgrade-guide claim that only held for errors-less activities.

propagateActivityFailure's E was unconstrained, so passing a ChildWorkflowError/ChildWorkflowCancelledError/ChildWorkflowNotFoundError (from executeChildWorkflow/startChildWorkflow) or a WorkflowCancelledError (from cancellableScope/nonCancellableScope) through it fell to the bare `throw error` fallback — a TaggedError, not a TemporalFailure, which Temporal retries as a workflow-task failure forever instead of failing the workflow. Add branches rethrowing each error's preserved cause, mirroring the existing ActivityError/ActivityCancelledError handling. ChildWorkflowNotFoundError has no cause (it fires before any Temporal call, on an undeclared child workflow name) — converted to ContractMisuseError instead so it still fails the execution terminally. Also corrects a stale comment claiming a defect's cause is rethrown "unchanged" when the code already classifies it through the same instanceof chain as an Err, and adds defect-channel test coverage (previously zero) with a discrimination proof. Also repoint propagation/routing/test/timeouts workflow fixtures at propagateActivityFailure's public re-export (../workflow.js) instead of the internal ../activity-failure.js path, so the worked examples exercise what six doc pages and the changeset tell users to import.

…-await hazard Close out the final review pass on the uniform-activity-result branch: - packages/worker/README.md taught the deleted convention verbatim ("Activities return plain values") and had two examples that now compile but silently discard or misuse the AsyncResult. - .agents/rules/handlers.md (this repo's own agent-guidance source of truth) still stated the deleted "activities without declared errors keep the throwing Promise shape" rule, plus three stale code blocks (a workflow example missing .value narrowing, an un-awaited activity call inside cancellableScope, and a nonCancellableScope/activity result both discarded). - upgrade-to-v8.md's migration checklist said cancellation only needs auditing on declared-error activities, contradicting the file's own "any activity call" prose a few hundred lines earlier. - The single most dangerous hazard on this branch — a bare `await context.activities.x(...)` compiles identically before and after 8.0 but now silently swallows the failure — had no coverage anywhere: not in the upgrade guide's prose, not in its checklist, and the changeset only mentioned swallowing in the context of ActivityCancelledError. Added a dedicated upgrade-guide section, a checklist item, and a changeset paragraph. - errors.md's ActivityError property table was missing the public `originalFailure` field (lands in the emitted .d.mts). Added the row and a cross-reference; mentioned it in the changeset. - worker-surface.md's propagateActivityFailure section now documents that it also covers child-workflow calls and cancellation scopes (see the paired fix(worker) commit). Every code snippet touched here was compiled against the real workspace packages (a scratchpad tsconfig with node_modules symlinked from packages/worker/node_modules) before landing — two of the "fixed" examples caught genuine follow-on type errors during that process (missing isDefect() narrowing) that a read-only review would have missed.

Three child-workflow.ts sites (input/output/signal-input validation) construct ChildWorkflowError without a cause. propagateActivityFailure's `throw error.cause ?? error` fallback rethrew the bare TaggedError there, which is not a TemporalFailure and stalls the workflow task instead of failing it — the exact hazard this helper exists to prevent. Give it the same ContractMisuseError treatment ChildWorkflowNotFoundError already gets. Corrects the JSDoc's false claim that ChildWorkflowError always mirrors ActivityError's cause handling, and trims the changeset's unconditional "no longer stalls the workflow" promise to reflect the actual behavior.

Copilot

Pull request overview

This PR hardens the worker-side workflow activity API by making every workflow-side activity call return AsyncResult, eliminating the prior “sometimes throws, sometimes returns a Result” bimodality and adding a dedicated helper (propagateActivityFailure) to preserve Temporal’s failure classification when callers want to “let it fail”.

Changes:

Make WorkflowInferActivity unconditional (AsyncResult for all activities) and route all activities through the result-shaped proxy wrapper.
Add propagateActivityFailure plus supporting error retention (ActivityError.originalFailure) to re-raise the original Temporal failure shape safely.
Migrate worker tests, examples, and docs to the new call convention; add targeted unit + in-process characterization coverage for propagation semantics.

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/worker/src/workflow.ts	Re-export helper/types; update JSDoc examples
packages/worker/src/errors.ts	Add `originalFailure`; update cancellation semantics docs
packages/worker/src/error-tags.ts	Update tag docs for uniform behavior
packages/worker/src/cancellation.ts	Update docs/examples for Result-shaped activity calls
packages/worker/src/activity-failure.ts	Implement `propagateActivityFailure`
packages/worker/src/activity-failure.spec.ts	Unit tests for re-raise behavior
packages/worker/src/activities-proxy.ts	Uniform `AsyncResult` activity proxy + error typing
packages/worker/src/activities-proxy.spec.ts	Update proxy tests for uniform Result shape
packages/worker/src/tests/timeouts.workflows.ts	Migrate fixture to propagate helper
packages/worker/src/tests/test.workflows.ts	Migrate test workflows to propagate helper
packages/worker/src/tests/routing.workflows.ts	Migrate routed workflow fixture
packages/worker/src/tests/propagation.workflows.ts	Add/migrate propagation fixtures
packages/worker/src/tests/propagation.inprocess.spec.ts	In-process characterization oracle for failure semantics
packages/worker/src/tests/propagation.contract.ts	Contract fixture for propagation characterization
packages/worker/src/tests/cancellation.inprocess.spec.ts	Update cancellation hazard description
packages/worker/src/tests/cancellation.contract.ts	Update contract fixture commentary for uniform Result
packages/worker/README.md	Migrate README workflow example
examples/README.md	Update example description text
examples/order-processing-worker/src/application/workflows.ts	Migrate example workflows to narrow/propagate explicitly
examples/order-processing-client/src/client.ts	Narrow signal send result instead of bare await
docs/tutorial/your-first-workflow.md	Update tutorial to propagate/narrow `AsyncResult`
docs/tutorial/adding-signals-and-queries.md	Update tutorial activity calls
docs/superpowers/specs/2026-08-04-uniform-activity-result-design.md	Add/record design spec
docs/superpowers/plans/2026-08-04-uniform-activity-result.md	Add/record implementation plan
docs/reference/worker-surface.md	Document uniform activity shape + propagation helper
docs/reference/errors.md	Update ActivityError/CancelledError docs + table
docs/index.md	Update landing page workflow snippet
docs/how-to/use-signals-queries-and-updates.md	Migrate activity calls in how-to
docs/how-to/upgrade-to-v8.md	Add migration guidance + hazards for uniform Result
docs/how-to/run-child-workflows.md	Update activity-call guidance and snippet
docs/how-to/model-domain-errors.md	Update narrative/tables for uniform Result
docs/how-to/handle-cancellation.md	Update cancellation guidance for uniform Result
docs/how-to/continue-as-new.md	Migrate examples to propagate helper
docs/explanation/workflow-determinism.md	Update examples for AsyncResult activity calls
docs/explanation/the-result-model.md	Update model/table for uniform activity Result
docs/explanation/nexus.md	Migrate example to propagate helper
.changeset/v8-audit-remediation.md	Clarify cancellation hazard scope
.changeset/uniform-activity-result.md	Add breaking-change changeset + migration notes
.agents/rules/handlers.md	Update internal rules/examples for uniform Result

Suppressed comments (1)

packages/worker/src/activities-proxy.spec.ts:102

Same issue as above: this cast narrows the error channel to ActivityError only, but the no-declared-errors path still includes ActivityCancelledError in the error union.

Copilot review on PR #370 flagged casts in activities-proxy.spec.ts narrowed to bare ActivityError for activities without a declared errors map, but ActivityErrorsFor's else-branch also includes ActivityCancelledError. Introduce NoDeclaredErrors mirroring that branch and use it at both sites instead of the wider ProxyError (which also carries AnyContractError, impossible in this case).

CONTRACT_ERROR_TAG and TECHNICAL_ERROR_TAG were re-exported a third time from errors-impl.ts, which is not a package entry point. Both public entries (`.` and `./errors`) already re-export the tags directly from error-tags.ts, so knip flagged the errors-impl.ts copy as unused. Removes the redundant export line only; the import it relied on stays in use by the TaggedError() calls in this file.

Copilot review on PR #370 flagged casts in activities-proxy.spec.ts narrowed to bare ActivityError for activities without a declared errors map, but ActivityErrorsFor's else-branch also includes ActivityCancelledError. Introduce NoDeclaredErrors mirroring that branch and use it at both sites instead of the wider ProxyError (which also carries AnyContractError, impossible in this case).

btravers added 18 commits August 4, 2026 01:40

docs: spec uniform AsyncResult for activity calls (workstream 4, part 1)

d5ba0f0

docs: plan uniform AsyncResult for activity calls

5f47bc9

test(worker): characterize activity-failure propagation before the Re…

eead9de

…sult change

feat(worker)!: return AsyncResult from every activity call

05d7f06

docs: migrate activity call sites and document the uniform Result con…

1d54842

…vention

docs(worker): capture scope Results and close remaining stale activit…

7fbd23a

…y claims

Copilot AI lite review requested due to automatic review settings August 4, 2026 07:41

Copilot started reviewing on behalf of btravers August 4, 2026 07:41 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Comment thread packages/worker/src/activities-proxy.spec.ts

btravers added 2 commits August 4, 2026 10:15

btravers merged commit 7ce642d into main Aug 4, 2026
12 checks passed

btravers deleted the feat/uniform-activity-result branch August 4, 2026 08:26

Navigation Menu

Uh oh!

feat(worker)!: return AsyncResult from every activity call - #370

feat(worker)!: return AsyncResult from every activity call#370
btravers merged 20 commits into
mainfrom
feat/uniform-activity-result

btravers commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

btravers commented Aug 4, 2026

The invariant this rests on

API

Breaking changes

The hazard that is not compiler-caught

Review

Verification

Follow-ups (pre-existing, none introduced here)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants