fix(worker): catch non-Promise thenables and hostile brand reads

Addresses both review comments on #357.

`bindQueryHandler` / `bindUpdateHandler`'s per-call sync guards used
`instanceof Promise` while the bind-time probe used `isThenable`. Standard
Schema types the async branch as `Promise<Result>`, but an implementation
may hand back any `PromiseLike` — that slipped past `instanceof Promise`,
and the helper then read `.issues` (undefined, i.e. "valid") and `.value`
(undefined) straight off the thenable, handing the handler an unvalidated
`undefined` instead of tripping `ContractMisuseError`. Both call sites now
share an `isAsyncValidation` helper that matches the probe's check and
detaches the thenable's settlement.

`_internal_declaredWorkflowName` read a symbol property off an arbitrary
module export, so a `Proxy` with a throwing `get` trap (or a throwing
getter) aborted the whole workflow-registration check with an unrelated
exception. The read is now guarded — anything that fails to yield a string
brand is simply "not a declared workflow".

Also fixes the red Integration Tests job: turbo ran the four
`test:integration` tasks in parallel, each spinning up its own Temporal +
Postgres testcontainer pair, and the contention pushed the client suite's
fixture setup past Vitest's 10s `hookTimeout` default. Serialize the task
(the workaround already documented in the PR description) and give the
integration projects a `hookTimeout` sized for fixture setup that opens
Temporal connections and builds a workflow bundle per worker.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: 2 people

examples/order-processing-worker/vitest.config.ts

@@ -5,6 +5,11 @@ export default defineConfig({
     globalSetup: "@temporal-contract/testing/global-setup",
     reporters: ["default"],
     setupFiles: ["./src/vitest.setup.ts"],
+    // These specs drive a real Temporal server through testcontainers, and
+    // fixture setup builds a workflow bundle per worker. Vitest's 5s test /
+    // 10s hook defaults are sized for unit tests, not for that.
+    testTimeout: 30_000,
+    hookTimeout: 60_000,
     coverage: {
       provider: "v8",
       reporter: ["text", "json", "json-summary", "html"],

package.json

@@ -16,7 +16,7 @@
     "prepare": "lefthook install",
     "release": "pnpm build && changeset publish",
     "test": "turbo run test",
-    "test:integration": "turbo run test:integration",
+    "test:integration": "turbo run test:integration --concurrency=1",
     "typecheck": "turbo run typecheck",
     "version": "changeset version"
   },

packages/client/vitest.config.ts

@@ -23,6 +23,12 @@ export default defineConfig({
           globalSetup: "@temporal-contract/testing/global-setup",
           include: ["src/**/__tests__/*.spec.ts"],
           testTimeout: 10_000,
+          // The per-test fixtures open three Temporal connections and build a
+          // workflow bundle per worker before the test body runs. Vitest's
+          // 10s hook default was never sized for that — on a loaded CI runner
+          // it trips before anything is actually wrong. `testTimeout` above
+          // still bounds the test body itself.
+          hookTimeout: 60_000,
           setupFiles: ["./src/vitest.setup.ts"],
         },
       },

packages/testing/vitest.config.ts

@@ -62,6 +62,10 @@ export default defineConfig({
           include: ["src/**/__tests__/*.spec.ts"],
           setupFiles: ["./src/vitest.setup.ts"],
           testTimeout: 30_000,
+          // Fixture setup opens Temporal connections and builds a workflow
+          // bundle before the test body runs; Vitest's 10s hook default is
+          // too tight for that on a loaded CI runner.
+          hookTimeout: 60_000,
         },
       },
     ],

packages/worker/src/handlers.spec.ts

@@ -77,6 +77,19 @@ const asyncStringSchema = {
   },
 };
 
+/**
+ * A validation result that is `PromiseLike` but NOT a `Promise` — the shape
+ * an `instanceof Promise` guard misses. The cast is the point: Standard
+ * Schema *types* the async branch as `Promise<Result>`, so only a runtime
+ * check can catch an implementation (a JS consumer, another realm's promise,
+ * a deferred wrapper) that returns a plain thenable instead.
+ */
+function bareThenable(value: unknown): Promise<{ value: unknown; issues: undefined }> {
+  // oxlint-disable-next-line unicorn/no-thenable -- the thenable IS the fixture: this test proves the sync guard catches a non-Promise PromiseLike
+  const thenable = { then: (resolve: (r: unknown) => void) => resolve({ value }) };
+  return thenable as unknown as Promise<{ value: unknown; issues: undefined }>;
+}
+
 const workflow = defineWorkflow({
   input: z.object({ id: z.string() }),
   output: z.object({}),
@@ -302,6 +315,37 @@ describe("bindQueryHandler", () => {
     expect(() => entry.impl(["x"])).toThrow(/validation must be synchronous/);
   });
 
+  it("the per-call guard catches a non-Promise thenable, not just a native Promise", () => {
+    // Standard Schema types the async signature as `Promise<Result>`, but an
+    // implementation may hand back any PromiseLike. An `instanceof Promise`
+    // guard would wave this through, and the helper would then read `.issues`
+    // (undefined → "valid") and `.value` (undefined) off the thenable and pass
+    // an unvalidated `undefined` to the handler.
+    captured.length = 0;
+    const thenableDodgingSchema = {
+      "~standard": {
+        version: 1 as const,
+        vendor: "test-thenable",
+        validate: (input: unknown) =>
+          typeof input === "symbol" ? { value: input, issues: undefined } : bareThenable(input),
+      },
+    };
+    const wf = defineWorkflow({
+      input: z.object({}),
+      output: z.object({}),
+      queries: {
+        progress: { input: thenableDodgingSchema, output: z.number() },
+      },
+    });
+    const handler = vi.fn().mockReturnValue(1);
+    bindQueryHandler(wf, "probe", "progress", handler as never);
+    const entry = captured.find((c) => c.kind === "query" && c.name === "progress")!;
+    expect(() => entry.impl(["x"])).toThrow(ContractMisuseError);
+    expect(() => entry.impl(["x"])).toThrow(/validation must be synchronous/);
+    // The handler never ran on the unvalidated payload.
+    expect(handler).not.toHaveBeenCalled();
+  });
+
   it("throws ContractMisuseError when the workflow has no queries block", () => {
     const noQueries = defineWorkflow({
       input: z.object({}),

packages/worker/src/handlers.ts

@@ -85,6 +85,31 @@ function isThenable(value: unknown): value is PromiseLike<unknown> {
   );
 }
 
+/**
+ * Per-call guard for the sync-only schema slots, kept shape-compatible with
+ * {@link assertSyncSchema}'s probe.
+ *
+ * Standard Schema types the async signature as `Promise<Result>`, but an
+ * implementation may legally hand back any `PromiseLike` — a wrapper, a
+ * deferred, a `then`-able from another realm. `instanceof Promise` misses
+ * those, and the caller would then read `.issues` (`undefined` → "no issues")
+ * and `.value` (`undefined`) straight off the thenable, silently handing the
+ * handler an unvalidated `undefined` instead of tripping
+ * {@link ContractMisuseError}. Matching the probe's `isThenable` closes that.
+ *
+ * A detected thenable's settlement is detached for the same reason the probe
+ * detaches its own: nothing awaits it, and a rejection would otherwise surface
+ * as an unhandled rejection while the {@link ContractMisuseError} is in flight.
+ */
+function isAsyncValidation(result: unknown): result is PromiseLike<unknown> {
+  if (!isThenable(result)) return false;
+  result.then(
+    () => undefined,
+    () => undefined,
+  );
+  return true;
+}
+
 /**
  * Bind-time probe for the sync-only schema slots (query input/output, update
  * input). Standard Schema permits `validate` to return a Promise (e.g. Zod
@@ -253,7 +278,7 @@ export function bindQueryHandler(
     const input = extractHandlerInput(args);
     const inputResult = queryDef.input["~standard"].validate(input);
 
-    if (inputResult instanceof Promise) {
+    if (isAsyncValidation(inputResult)) {
       // oxlint-disable-next-line unthrown/no-throw -- sanctioned ContractMisuseError model: non-retryable ApplicationFailure Temporal must see thrown (CLAUDE.md rule 2 exception)
       throw new ContractMisuseError(
         `Query "${queryName}" validation must be synchronous. Use a schema library that supports synchronous validation for queries.`,
@@ -267,7 +292,7 @@ export function bindQueryHandler(
     const result = handler(inputResult.value);
 
     const outputResult = queryDef.output["~standard"].validate(result);
-    if (outputResult instanceof Promise) {
+    if (isAsyncValidation(outputResult)) {
       // oxlint-disable-next-line unthrown/no-throw -- sanctioned ContractMisuseError model: non-retryable ApplicationFailure Temporal must see thrown (CLAUDE.md rule 2 exception)
       throw new ContractMisuseError(
         `Query "${queryName}" output validation must be synchronous. Use a schema library that supports synchronous validation for queries.`,
@@ -360,7 +385,7 @@ export function bindUpdateHandler(
       // worth surfacing.
       const input = extractHandlerInput(args);
       const inputResult = updateDef.input["~standard"].validate(input);
-      if (inputResult instanceof Promise) {
+      if (isAsyncValidation(inputResult)) {
         // oxlint-disable-next-line unthrown/no-throw -- sanctioned ContractMisuseError model: non-retryable ApplicationFailure Temporal must see thrown (CLAUDE.md rule 2 exception)
         throw new ContractMisuseError(updateInputMustBeSynchronousMessage(updateName));
       }
@@ -390,7 +415,7 @@ export function bindUpdateHandler(
         const input = extractHandlerInput(args);
         const inputResult = updateDef.input["~standard"].validate(input);
 
-        if (inputResult instanceof Promise) {
+        if (isAsyncValidation(inputResult)) {
           // oxlint-disable-next-line unthrown/no-throw -- sanctioned ContractMisuseError model: non-retryable ApplicationFailure Temporal must see thrown (CLAUDE.md rule 2 exception)
           throw new ContractMisuseError(updateInputMustBeSynchronousMessage(updateName));
         }

packages/worker/src/workflow-brand.spec.ts

@@ -0,0 +1,60 @@
+/**
+ * Coverage for the `declareWorkflow` brand reader used by `TypedWorker`'s
+ * workflow-registration check.
+ *
+ * The reader is fed arbitrary module exports, so every assertion here is
+ * about it staying total: non-functions, unbranded functions, non-string
+ * brands, and — the case that matters most — exports whose property read
+ * itself throws. A throwing read must degrade to "not a declared workflow",
+ * not abort the registration check with an unrelated exception.
+ */
+import { describe, expect, it } from "vitest";
+
+import { DECLARED_WORKFLOW_BRAND, _internal_declaredWorkflowName } from "./workflow-brand.js";
+
+describe("_internal_declaredWorkflowName", () => {
+  it("returns the declared name for a branded function", () => {
+    const branded = Object.assign(() => undefined, {
+      [DECLARED_WORKFLOW_BRAND]: "orderWorkflow",
+    });
+    expect(_internal_declaredWorkflowName(branded)).toBe("orderWorkflow");
+  });
+
+  it("returns undefined for non-functions and unbranded functions", () => {
+    expect(_internal_declaredWorkflowName(undefined)).toBeUndefined();
+    expect(_internal_declaredWorkflowName(null)).toBeUndefined();
+    expect(_internal_declaredWorkflowName("orderWorkflow")).toBeUndefined();
+    expect(
+      _internal_declaredWorkflowName({ [DECLARED_WORKFLOW_BRAND]: "notAFunction" }),
+    ).toBeUndefined();
+    expect(_internal_declaredWorkflowName(() => undefined)).toBeUndefined();
+  });
+
+  it("returns undefined when the brand is present but not a string", () => {
+    const branded = Object.assign(() => undefined, { [DECLARED_WORKFLOW_BRAND]: 42 });
+    expect(_internal_declaredWorkflowName(branded)).toBeUndefined();
+  });
+
+  it("returns undefined instead of propagating a throwing getter", () => {
+    const hostile = () => undefined;
+    Object.defineProperty(hostile, DECLARED_WORKFLOW_BRAND, {
+      get() {
+        // oxlint-disable-next-line unthrown/no-throw -- test double: simulates a module export whose property read throws
+        throw new Error("hostile getter");
+      },
+    });
+    expect(() => _internal_declaredWorkflowName(hostile)).not.toThrow();
+    expect(_internal_declaredWorkflowName(hostile)).toBeUndefined();
+  });
+
+  it("returns undefined instead of propagating a Proxy `get` trap that throws", () => {
+    const hostile = new Proxy(() => undefined, {
+      get() {
+        // oxlint-disable-next-line unthrown/no-throw -- test double: simulates a Proxy export with a throwing get trap
+        throw new Error("hostile trap");
+      },
+    });
+    expect(() => _internal_declaredWorkflowName(hostile)).not.toThrow();
+    expect(_internal_declaredWorkflowName(hostile)).toBeUndefined();
+  });
+});

packages/worker/src/workflow-brand.ts

@@ -22,10 +22,21 @@ export const DECLARED_WORKFLOW_BRAND = Symbol.for("temporal-contract.declareWork
  * with, or `undefined` for anything else. Used by `TypedWorker`'s
  * workflow-registration completeness check.
  *
+ * The candidate is an arbitrary module export, so the property read itself is
+ * untrusted: a `Proxy` with a throwing `get` trap, or a function carrying a
+ * throwing getter, would otherwise abort the whole registration check with an
+ * unrelated exception. Anything that fails to yield a plain string brand —
+ * including by throwing — is simply "not a declared workflow".
+ *
  * @internal
  */
 export function _internal_declaredWorkflowName(candidate: unknown): string | undefined {
   if (typeof candidate !== "function") return undefined;
-  const brand = (candidate as { [DECLARED_WORKFLOW_BRAND]?: unknown })[DECLARED_WORKFLOW_BRAND];
+  let brand: unknown;
+  try {
+    brand = (candidate as { [DECLARED_WORKFLOW_BRAND]?: unknown })[DECLARED_WORKFLOW_BRAND];
+  } catch {
+    return undefined;
+  }
   return typeof brand === "string" ? brand : undefined;
 }

packages/worker/vitest.config.ts

@@ -24,6 +24,11 @@ export default defineConfig({
           include: ["src/**/__tests__/*.spec.ts"],
           exclude: ["src/**/__tests__/*.inprocess.spec.ts"],
           testTimeout: 10_000,
+          // Fixture setup opens Temporal connections and builds a workflow
+          // bundle per worker before the test body runs; Vitest's 10s hook
+          // default is too tight for that on a loaded CI runner. The test
+          // body itself is still bounded by `testTimeout` above.
+          hookTimeout: 60_000,
           setupFiles: ["./src/vitest.setup.ts"],
         },
       },