test!: prove determinism and money-safety invariants by effect - #360
Merged
Conversation
Workstream 2 of the production-hardening effort. Scopes to invariants the library could silently break, rather than new API. Leads with the finding that reframes the workstream: `nonRetryable` is asserted only as a field value on the wire failure, never as behavior. That is a call-shape assertion in exactly the sense workstream 1 set out to eliminate, and it survived because those assertions were added DURING workstream 1 as fixes restoring a dropped property — restoring a property is not the same as proving a behavior. Replay coverage is harvested rather than enumerated: the in-process tier already produces real histories across every construct, so a rig that records started workflow IDs alongside the bundle gives near-total coverage and makes future tests covered automatically. Idempotency surface is explicitly excluded and deferred to workstream 4 — the SDK's reuse/conflict policies already pass through, so that work is API design rather than invariant proving.
Eight tasks: the testRig replay-harvest helper, five migration tasks moving the in-process specs onto it, retry terminality proven by attempt count, timeout forwarding proven via Context.current().info, and final verification. Records API facts verified against the installed SDK so implementers do not re-derive them: onTestFinished may be called from inside a test body (which is what lets the rig register its own teardown), ReplayWorkerOptions accepts workflowBundle, the terminal-status union spells CANCELLED with two Ls, and ActivitiesHandler lives on the /activity subpath rather than /worker. Two constraints are called out because getting them wrong would look like a tidy-up: the rig must spread `activities` conditionally (an absent key differs from undefined), and it must NOT scope the task queue, because a same-workflow continue-as-new has to land on the contract's static queue. Every migration task carries an explicit triage rule: a DeterminismViolation in shipped code is a finding that stops the task, never something to allowlist or adjust the test around.
…hazards The rule banned `Date.now()`, `Math.random()` and `setTimeout` with a rationale implying the sandbox would not protect you. Verified against the installed SDK (`@temporalio/workflow/lib/global-overrides.js`): all three are patched before workflow code runs. `Date.now()` returns `getActivator().now` (workflow time), `Math.random` is a seeded deterministic stream, and `setTimeout` becomes a durable Temporal timer wrapped in a cancellation scope. `WeakRef` and `FinalizationRegistry` throw DeterminismViolationError outright. `crypto` is neither injected by the SDK nor inherited by a bare vm context (checked empirically on Node 24), so it reference-errors rather than silently producing nondeterminism. Restructures the rule into three tables by failure mode — patched-but-different semantics, blocked loudly, and genuinely unprotected — because which column a hazard sits in determines whether a mistake surfaces as a crash, a surprise, or silent corruption. The real risk was under-emphasised: `process.env`, module-level mutable state, and `import.meta` are the things nothing patches. Those now lead. Also updates the workstream-2 plan's Global Constraints, which had copied the old rationale verbatim.
The section still cited `console.log({ now: Date.now() })` as non-deterministic,
which the corrected tables directly refute — `Date.now()` returns workflow time
and is stable across replays. Swapped it for `process.env.REGION`, which is a
genuine example: that value really can differ between the worker that ran the
workflow and the worker that replays it.
Also reframed the escape hatch itself. It previously read as "for when you need
non-determinism", which is not what a LocalActivity is for — it is for the
things the sandbox cannot provide at all.
Builds the worker + client pair every in-process test needs and registers an onTestFinished hook that replays the Temporal history of every execution the client started, so replay-determinism coverage follows the existing in-process test tier automatically instead of rotting as an enumerated list.
- Fail loud (throw) when a start call's options bag carries no string workflowId, instead of silently recording nothing and letting onTestFinished iterate an empty set. Extracted as the pure, unit-tested extractStartedWorkflowId. - Tolerate WorkflowNotFoundError from describe() in teardown: a start call that fails client-side before dispatch (e.g. contract validation) records an id for an execution the server never created, and describe() must not treat that as a real failure. - Walk continue-as-new/retry/cron chains backward via each run's WorkflowExecutionStarted.continuedExecutionRunId and replay every run, not just the latest — the prior finding-final-run-only design silently skipped the histories most worth replaying and left CONTINUED_AS_NEW effectively dead in TERMINAL_STATUSES. Also: dedupe startedIds with a Set, mark REPLAY_SKIP_ALLOWLIST Readonly, and add JSDoc to the two bare typedoc entry points.
Replace the manual TypedWorker/TypedClient construction and inline replay loop with testRig, which harvests started workflow histories and replays them in onTestFinished. Proved the teardown actually performs the replay by temporarily pointing test-rig.ts's runReplayHistory at an unrelated workflow module and observing a real ReplayError, then reverted. Also alias @temporal-contract/client and @temporal-contract/worker/worker to source in packages/worker/vitest.config.ts, and stop externalizing @temporal-contract/testing. testRig's built test-rig.mjs imports those two peer specifiers at the top level; peerDependencies (not devDependencies, which would create a real client<->testing cycle) can't resolve from packages/testing's own node_modules inside this workspace, since pnpm symlinks @temporal-contract/testing straight to its source directory and Vitest externalizes prebuilt node_modules dependencies to Node's native loader, bypassing Vite aliases. Mirrors the same aliasing technique packages/testing/vitest.config.ts already uses for its own peers.
Tried dependenciesMeta.injected: true on @temporal-contract/testing in packages/worker/package.json first, per review — it hard-links testing into worker's own node_modules so Node's realpath walk would find client/worker as siblings, with no vitest config needed at all. It doesn't work: resolving testing's peer on @temporal-contract/worker (this very package, which — being a peer, not a workspace dependency — never self-references) fell through to the registry and tripped minimumReleaseAge's supply-chain-maturity gate ([ERR_PNPM_NO_MATURE_MATCHING_VERSION], naming @temporal-contract/contract@8.0.0-beta.4 and @temporal-contract/worker@8.0.0-beta.4). Confirmed by toggling the package.json change on and off against an otherwise-clean pnpm install: fails only with injected: true present. Reverted that package.json change entirely. Falling back to the alias, but scoped to the integration-inprocess project only — the previous commit had put it on both integration and integration-inprocess, which silently swapped 10 Docker-tier spec files (including routing.spec.ts and worker.spec.ts, unrelated to this task) from exercising @temporal-contract/client's built dist output to its source, retiring the only place that dist ever ran under test. Docker's integration tier doesn't consume testRig and had no resolution problem to begin with. Also corrected the in-code comment: server.deps.inline has no counterpart in packages/testing/vitest.config.ts. That config aliases bare specifiers imported from its own source .ts files, which Vite/esbuild resolves (alias included) without needing to be told not to externalize anything. Here the entry point is a prebuilt .mjs under @temporal-contract/testing, which Vitest externalizes to Node's native loader by default, bypassing the alias entirely — server.deps.inline is what forces it through Vite's resolver instead, and that half of the technique is new, not mirrored from testing's config.
Migrates handlers.inprocess.spec.ts and activity-options.inprocess.spec.ts onto testRig's automatic replay-determinism coverage. Adds two REPLAY_SKIP_ALLOWLIST entries for handlers.workflows.ts's probeEdgeCases and transformWorkflow, which block on condition(() => false) by design. Also fixes a latent testRig type gap this migration exposed: tsc silently collapsed ContractClient<TContract> to `any` (masked by skipLibCheck) because @temporal-contract/client/contract/worker are peerDependencies of @temporal-contract/testing with no node_modules route from testing's own dist directory — the same resolution gap Task 2's vitest.config.ts alias fixed for Vitest, but tsc doesn't read that config. Adds the tsc-level equivalent via packages/worker/tsconfig.json paths, scoped to that package only and invisible to Vite (no vite-tsconfig-paths plugin), so it doesn't touch runtime resolution or the Docker integration tier.
… allowlist keys Review of the previous commit found the tsconfig paths fix was 3/4 complete: it mapped @temporal-contract/client, /contract, and /worker/activity, but missed /worker/worker — the fourth specifier testRig's built d.mts actually imports. TypedWorker (and therefore worker.raw.runUntil()'s return type) was silently `any`, invisible because runUntil's callback in activity-options.inprocess.spec.ts takes no arguments, so noImplicitAny never fired on it. Five assertions (outcome.outcome, .contractTimeout, .usesDefault, .globalTimeout, .flaky) were statically unchecked as a result. Rewrites packages/worker/tsconfig.json's paths to mirror packages/testing/tsconfig.json's already-proven pattern: all four specifiers mapped to sibling source (not a client/contract-to-dist, worker-to-src mix), rootDir widened to ".." to allow it. Verified with `tsc --skipLibCheck false` (zero @temporal-contract errors remain) and by temporarily probing the five assertion sites with typo'd property names, confirming tsc now names their real structural types instead of allowing anything through. Also narrows the handlers-wire- REPLAY_SKIP_ALLOWLIST prefix to the three exact (non-counter-suffixed) workflow IDs it was standing in for, so a future handlers-wire-* workflow that hangs by accident isn't silently swept into the same entry.
Both files replace the TypedWorker.create + TypedClient.create + .for(contract)
trio with testRig(testEnv, { contract, bundle, activities }), mechanically,
per the established pattern. No assertions changed, no REPLAY_SKIP_ALLOWLIST
entries needed: every cancellation history and every continue-as-new chain
replayed cleanly.
child-wire and rehydration take the full mechanical migration. registration is left untouched (every test depends on workflowsPath to exercise verifyWorkflowRegistration, incompatible with the rig's bundle-only construction). time-skipping only migrates its one test with no special worker/client options and that starts a workflow — the rest test creation mechanics directly (defects, custom WorkerOptions, interceptors) and stay manual, same as registration.
Every existing assertion about nonRetryable reads raw.nonRetryable === true off the wire failure -- a call-shape assertion that would survive the flag being dropped between the wire and Temporal's retry decision. Add retry.contract.ts/retry.workflows.ts/retry.inprocess.spec.ts: two declared errors identical but for nonRetryable, an activity that reports Context.current().info.attempt (Temporal's own counter, not a local variable), and assertions on the surfaced attempt count. The terminal case must show exactly one attempt; the retryable case must exhaust maximumAttempts: 3, proving Temporal actually retried.
…ETHODS
Fixes two items from the final whole-branch review:
- REPLAY_SKIP_ALLOWLIST was a module constant baked into the published
test-rig, silently skipping any external consumer's workflow IDs that
happened to match this repo's fixture prefixes, and pointing them at a
path they can't reach on failure. It's now a `replaySkipAllowlist` option
on RigOptions, defaulting to {}, with the skip error naming the caller's
own option instead of a path in this monorepo.
- START_METHODS was a hardcoded, unguarded string set: a ContractClient
rename or a fourth start method would make the rig's Proxy silently stop
intercepting it, with the whole replay-coverage tier going green while
proving nothing. testRig now asserts every START_METHODS name resolves to
a real function on the bound client, and a new unit test pins the set
against ContractClient's actual runtime method surface.
Companion to the previous commit's replaySkipAllowlist option: the four entries that used to live in @temporal-contract/testing/test-rig move to this repo's own fixtures, since they name workflow IDs specific to handlers.workflows.ts. Only handlers.inprocess.spec.ts's four testRig(...) calls that actually start those non-terminal executions (handlers-probe-edge-cases, handlers-wire-signal/query/update) now pass replaySkipAllowlist; the file's other 11 testRig(...) calls, and every call site in every other *.inprocess.spec.ts file, are untouched.
9 of 59 in-process tests ran off the rig, exempted only in prose in an archived report — nothing stopped a future *.inprocess.spec.ts from hand-rolling TypedWorker.create + TypedClient.create (the pattern still visible in this file's own JSDoc examples) and silently getting zero replay coverage. Mirrors no-sdk-mocks.spec.ts's shape (workspace walk, shrink-only reason-carrying allowlist, positive control, stale-entry check), but checks per test rather than per file: each file is split into blocks on its `it(` lines, and every block must contain a testRig( call. A file-level "contains testRig( anywhere" check would have let time-skipping.inprocess.spec.ts (1 rig test, 4 manual) pass without an allowlist entry, which doesn't match what's actually covered. Allowlist has exactly two entries: registration.inprocess.spec.ts (every test must pass workflowsPath directly — testRig's bundle-only RigOptions always skips verifyWorkflowRegistration) and time-skipping.inprocess.spec.ts (2 tests assert on the creation Result itself; 2 need interceptors / WorkerOptions passthrough RigOptions lacks).
On a branch whose value is "the assertion means what it says", a comment overstating what a test proves is the same defect in prose: - retry.contract.ts claimed the activity chooses its error from input; both test handlers hardcode their error and never read _input, and mode only threads client -> workflow -> activity unread. - timeouts.inprocess.spec.ts / timeouts.workflows.ts claimed activityOptionsByName "wins — most specific layer". The three asserted keys are disjoint across all three merge layers, so reversing the spread order would leave the test green — it proves forwarding, not precedence (precedence is covered separately by activity-options.contract.ts). - time-skipping.inprocess.spec.ts had 1 rig test among 4 manual ones with no in-file explanation; added an inline comment naming why, in the style of rehydration.inprocess.spec.ts's own hybrid explanation.
packages/testing/package.json gained "./test-rig" in exports plus a typedoc.json entry point on this branch, with no changeset for it. Matches testing-workflow-bundle-export.md's style: minor bump, one paragraph naming the exported symbols and what the subpath provides, including the replaySkipAllowlist option landing in the same release.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens temporal-contract’s test suite by proving determinism and “money-safety” invariants by effect (Temporal-observed behavior), and by making replay-determinism coverage automatic across the in-process test tier via a shared testRig.
Changes:
- Add and publish
@temporal-contract/testing/test-rig, which builds aTypedWorker+ typed client pair and replays histories for every started workflow in anonTestFinishedhook. - Add new in-process invariant tests proving (a)
nonRetryablevia attempt count and (b) timeout forwarding viaContext.current().info. - Migrate many worker in-process specs to the rig and add guards/allowlists to prevent silent replay-coverage regressions.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/worker/vitest.config.ts | Adds Vite/Vitest aliasing + inline-deps routing so @temporal-contract/testing’s built rig can resolve peer deps inside the monorepo. |
| packages/worker/tsconfig.json | Expands rootDir and adds paths mappings so tsc resolves rig-imported peer types to sibling source. |
| packages/worker/src/tests/timeouts.workflows.ts | Adds workflow fixture that exercises all three activity-timeout merge layers. |
| packages/worker/src/tests/timeouts.inprocess.spec.ts | New effect-based timeout forwarding test using Context.current().info. |
| packages/worker/src/tests/timeouts.contract.ts | New contract fixture defining heartbeat timeout and output schema for timeout assertions. |
| packages/worker/src/tests/time-skipping.inprocess.spec.ts | Migrates one test to testRig (others intentionally remain off-rig). |
| packages/worker/src/tests/retry.workflows.ts | Adds workflow fixture that returns retry outcomes rather than rethrowing (avoids time-skipping hangs). |
| packages/worker/src/tests/retry.inprocess.spec.ts | New effect-based retry tests proving nonRetryable via activity attempt counts. |
| packages/worker/src/tests/retry.contract.ts | New contract fixture defining retryable vs non-retryable declared errors. |
| packages/worker/src/tests/replay.inprocess.spec.ts | Switches replay determinism coverage to the rig’s automatic replay hook. |
| packages/worker/src/tests/rehydration.inprocess.spec.ts | Migrates worker/client construction to testRig where compatible. |
| packages/worker/src/tests/handlers.inprocess.spec.ts | Migrates to testRig and introduces per-spec replay-skip allowlisting for non-terminal workflows. |
| packages/worker/src/tests/handlers-replay-skip-allowlist.ts | New shrink-only allowlist for intentionally non-terminal handler fixtures. |
| packages/worker/src/tests/continue-as-new.inprocess.spec.ts | Migrates to testRig for automatic replay coverage across continue-as-new chains. |
| packages/worker/src/tests/child-wire.inprocess.spec.ts | Migrates to testRig for replay coverage on child-workflow wire tests. |
| packages/worker/src/tests/cancellation.inprocess.spec.ts | Migrates to testRig for automatic replay determinism coverage on cancellation scenarios. |
| packages/worker/src/tests/activity-options.inprocess.spec.ts | Migrates to testRig for replay coverage while asserting Temporal-materialized options. |
| packages/testing/typedoc.json | Adds src/test-rig.ts to docs entry points. |
| packages/testing/src/test-rig.ts | Implements testRig + replay harvesting, chain-walking, and runtime/compile-time guards. |
| packages/testing/src/test-rig.spec.ts | Adds unit tests pinning rig assumptions (terminal statuses, start-method surface, id extraction, etc.). |
| packages/testing/src/inprocess-specs-use-rig.spec.ts | Adds meta-test enforcing testRig( usage in in-process specs (with allowlist). |
| packages/testing/package.json | Exports new ./test-rig subpath and adds it to the build entry list. |
| docs/superpowers/specs/2026-08-02-determinism-invariants-design.md | Adds the approved design spec for determinism/money-safety invariant proofs. |
| docs/superpowers/plans/2026-08-02-determinism-invariants.md | Adds the detailed implementation plan for the workstream. |
| AGENTS.md | Updates determinism guidance to reflect sandbox patching vs truly unprotected hazards. |
| .changeset/testing-test-rig-export.md | Changeset publishing the new testing subpath export. |
| .agents/rules/workflow-determinism.md | Updates determinism rule doc with patched/blocked/unprotected taxonomy and clarified guidance. |
Suppressed comments (1)
packages/testing/src/inprocess-specs-use-rig.spec.ts:120
- The stale-allowlist check uses
block.includes("testRig("), which has the same false-negative risk as the main guard (a comment/string can satisfy it). It should use the same comment-stripped call check so allowlist entries are accurately detected as stale.
for (const rel of Object.keys(ALLOWLIST)) {
const source = await readFile(join(WORKSPACE_ROOT, rel), "utf8").catch(() => "");
const stillOffRig = testBlocks(source).some((block) => !block.includes("testRig("));
if (!stillOffRig) stale.push(rel);
…guard All four are the same shape the guards exist to prevent — machinery that passes while checking nothing. Reported by Copilot on #360; the first was independently flagged as a residual gap by this branch's own final review. - TEST_START only matched `it(`. A spec written with `test(` or `it.each(` yielded zero blocks, so the offender loop never ran and the file passed enforcing nothing. Widened to any Vitest spelling, and backed by a block-level positive control: the file-count assertion proved the corpus walk found files, not that block-splitting found tests inside them. - The `testRig(` check was a raw substring match, so a comment merely mentioning the rig satisfied it. Comments are now stripped first. - testDescription only parsed `it("…")`, so a `test(` offender was reported as "(description not found)" — surfaced by the proof for the fix above. - skipReasonFor returned the first matching prefix by Object.entries order, making overlapping entries order-dependent and letting a broad prefix shadow a deliberately narrower one. Longest match now wins. - extractStartedWorkflowId interpolated JSON.stringify(bag) into its error. A BigInt or circular bag makes that throw, replacing the guard's actionable diagnostic with an unrelated TypeError. Safe fallback added. Each fix was proven by planting the failure it claims to catch and observing it, then reverting: a `test(`-spelled spec, a comment-only mention, and reverting each test-rig change with its new unit tests failing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Workstream 2 of 4 in the production-hardening effort. Implements
docs/superpowers/specs/2026-08-02-determinism-invariants-design.md. Follows #359 (mock-free test architecture).Proves three families of invariant by effect — invariants this library could silently break, where the user's workflow then misbehaves in a way their own tests would not catch.
The finding that reframed the workstream
nonRetryablewas asserted only as a field value on the wire failure:That would survive the flag being dropped between the wire and Temporal's retry decision. Nothing anywhere proved Temporal actually stopped retrying — for a library whose most money-relevant declaration is "this error is terminal".
It persisted precisely because of #359: those assertions were added during that workstream as fixes restoring a dropped property. Restoring a property is not the same as proving a behavior.
What changed
nonRetryableContext.current().info.attempt— exactly 1 vs the declaredmaximumAttempts: 3startToCloseTimeoutproven by waiting for it to fire;heartbeatTimeoutasserted nowhere in the repoContext.current().info— what Temporal actually materializedThe replay rig
testRigreplaces the worker+client construction already present in every in-process test. Its client records started workflow IDs; anonTestFinishedhook fetches each execution's history and replays it. Coverage is therefore automatic for future tests, not an enumerated list that rots.It walks continue-as-new chains backwards via
continuedExecutionRunIdand replays every run — the plan originally said final-run-only was "correct and sufficient", which was wrong: the earlier runs are the ones holding the continue-as-new command. One chain turned out to be cross-contract, replaying a different workflow type against the same bundle.Results
turbo run typecheck lint testNo
DeterminismViolationErrorsurfaced in shipped code. That is a real result, not a null one — it was the outcome that would have outranked the rest of the workstream.Reviewer notes
Wall clock: the in-process tier is ~7s wall / ~38s serial-equivalent across ~6 forks. Replay adds roughly 1s of wall time. An earlier draft of this description claimed a 5× speedup — that was two different quantities being compared, and it is not true.
Nine of 59 tests are not on the rig, both cases sanctioned and now enforced by a shrink-only allowlist rather than prose:
registration.inprocess.spec.tsmust keepworkflowsPathorverifyWorkflowRegistrationis skipped (that check is the file's whole subject), and 4 of 5time-skipping.inprocess.spec.tstests assert on creation mechanicsRigOptionscannot express.The rig's one catastrophic failure mode — silently reporting coverage it does not have — is defended at three independent layers: a runtime guard that
START_METHODSstill resolves on the bound client, a reflection-based test pinning that set againstContractClient's real surface, and a corpus guard requiringtestRig(in every in-process spec. All three were observed failing before being kept.@temporal-contract/testing/test-rigis a new published subpath.replaySkipAllowlistis a caller parameter defaulting to{}— an earlier draft hardcoded this repo's fixture IDs, which would have silently skipped a consumer's workflow whose ID happened to share a prefix.Known follow-ups:
RigOptionslacksinterceptors/workerOptionspassthrough (would recover 4 more executions); the chain-walk pointer extraction has no unit test (it is proven by integration, twice independently, but has no regression guard); child-workflow executions are not harvested, since a client-side recorder cannot see workflows started from inside a workflow.Verification
pnpm turbo run typecheck lint test, thenpnpm turbo run test:integration --concurrency=1(Docker). Do not pipelineknipimmediately after a build in one shell line —tsdown --cleandeletesdist/before re-emitting, and knip reads workspace imports through it.