Fix sdkdev integration failures: gate REGION_DURABLE, cover DURABLE, pin OSS image

.github/workflows/agent-e2e.yml

@@ -18,8 +18,19 @@ concurrency:
   group: agent-e2e-${{ github.ref }}
   cancel-in-progress: true
 
+# Deliberately NOT the same knob as the integration-tests-oss job in
+# pull_request.yml, despite the similar name. That job pins a *Docker image
+# tag* (conductoross/conductor on Docker Hub), defaulted in
+# scripts/docker-compose-oss.yaml and overridable via the
+# E2E_TEST_OSS_CONDUCTOR_VERSION org variable. This is a *Maven Central
+# artifact version* for the conductor-server boot JAR, which is a different
+# registry with a different set of published versions -- notably it has no
+# `latest`, which is what that org variable currently holds. It also carries a
+# constraint that job does not: the /agent/* control plane and the
+# TaskDef.runtimeMetadata persistence these suites exercise only exist from
+# 3.32.0-rc.8 onward. So it is pinned independently and bumped deliberately.
 env:
-  CONDUCTOR_OSS_VERSION: ${{ vars.CONDUCTOR_SERVER_VERSION || '3.32.1' }}
+  CONDUCTOR_OSS_VERSION: ${{ vars.CONDUCTOR_SERVER_VERSION || '3.32.1' }} # pinned conductor-oss release — see note above
 
 jobs:
   agent-e2e:

.github/workflows/pull_request.yml

@@ -6,6 +6,11 @@ on:
       - main
   pull_request:
   workflow_dispatch:
+    inputs:
+      oss_conductor_version:
+        description: 'OSS Conductor image tag (falls back to E2E_TEST_OSS_CONDUCTOR_VERSION org var, then to a pinned default on fork PRs)'
+        required: false
+        type: string
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
@@ -217,6 +222,33 @@ jobs:
       CONDUCTOR_RETRY_SERVER_ERRORS: "true"
       HTTPBIN_SERVICE_HOSTNAME: httpbin
     steps:
+      # OSS_CONDUCTOR_VERSION is resolved here rather than in the job `env` so
+      # that the two ways it can come back empty get different treatment:
+      #
+      #   - Fork PR: GitHub withholds org/repo variables from pull_request runs
+      #     on forks exactly as it withholds secrets, so vars.* is always "" for
+      #     an outside contributor (observed in csharp-sdk#178). This job needs
+      #     no secrets, only a tag, so leave the var unset and let the default
+      #     baked into the `image:` line of scripts/docker-compose-oss.yaml
+      #     apply. That is the same tag a plain local run of
+      #     scripts/run-integration-oss.sh gets, and the one place it is
+      #     written -- no second copy to drift out of sync here.
+      #   - Anything else: the org variable is genuinely missing or its
+      #     repository access policy no longer covers this repo. Fail loudly
+      #     rather than silently drifting onto the default.
+      - name: Resolve OSS Conductor version
+        env:
+          REQUESTED_VERSION: ${{ inputs.oss_conductor_version || vars.E2E_TEST_OSS_CONDUCTOR_VERSION }}
+          IS_FORK_PR: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }}
+        run: |
+          if [ -n "$REQUESTED_VERSION" ]; then
+            echo "OSS_CONDUCTOR_VERSION=${REQUESTED_VERSION}" >> "$GITHUB_ENV"
+          elif [ "$IS_FORK_PR" = "true" ]; then
+            echo "::notice::Fork PR: org variables are withheld, falling back to the default tag in scripts/docker-compose-oss.yaml"
+          else
+            echo "::error::No Conductor OSS image tag resolved. Set the E2E_TEST_OSS_CONDUCTOR_VERSION organization variable (and ensure its repository access policy includes this repo), or pass the oss_conductor_version input via workflow_dispatch."
+            exit 1
+          fi
       - name: Checkout
         uses: actions/checkout@v4
       - name: Set up Node
@@ -235,6 +267,20 @@ jobs:
       - name: Install Dependencies
         if: steps.cache.outputs.cache-hit != 'true'
         run: npm ci
+      # `docker compose up` only pulls an image when it is missing locally. On a
+      # GitHub-hosted runner the VM is ephemeral and starts with no cached copy
+      # of this image, so `up` would pull anyway and this step is redundant
+      # today. It is here deliberately: it costs no extra network pull (`up`
+      # then finds the image locally), it separates "couldn't pull the image"
+      # from "the stack didn't come up" into two distinct red steps, and it is
+      # what keeps a mutable tag from going stale if this job ever moves to a
+      # self-hosted runner with a warm Docker daemon -- the same reason
+      # scripts/run-integration-oss.sh pulls. It also prints the tag actually in
+      # use, which for a fork PR comes from the compose file's default.
+      - name: Pull Conductor OSS image
+        run: |
+          echo "Using $(docker compose -f scripts/docker-compose-oss.yaml config --images conductor-server)"
+          docker compose -f scripts/docker-compose-oss.yaml pull conductor-server
       - name: Start Conductor OSS stack
         run: docker compose -f scripts/docker-compose-oss.yaml up -d
       - name: Wait for Conductor to be healthy

scripts/docker-compose-oss.yaml

@@ -4,9 +4,16 @@
 #
 # The Conductor server reaches httpbin over the compose network at
 # http://httpbin:8081, which matches HTTPBIN_SERVICE_HOSTNAME=httpbin.
+#
+# The `image:` default below is the SINGLE place the Conductor OSS image tag is
+# written. Everything that does not override OSS_CONDUCTOR_VERSION lands on it:
+# a plain `scripts/run-integration-oss.sh` run, and the integration-tests-oss
+# job on a fork PR (where GitHub withholds org variables). Overrides are the
+# script's --version flag and, in CI, the E2E_TEST_OSS_CONDUCTOR_VERSION org
+# variable or a workflow_dispatch input. Bump the tag here and both follow.
 services:
   conductor-server:
-    image: conductoross/conductor:latest
+    image: conductoross/conductor:${OSS_CONDUCTOR_VERSION:-3.32.3}
     environment:
       - CONFIG_PROP=config-postgres.properties
     ports:

scripts/run-integration-oss.sh

@@ -12,12 +12,13 @@
 # scripts/oss-test-run.log, override with -l|--log) so it can be shared later.
 #
 # Usage:
-#   scripts/run-integration-oss.sh [-t|--test <path|pattern>] [-l|--log <file>] [--keep-up] [-- jest args]
+#   scripts/run-integration-oss.sh [-t|--test <path|pattern>] [-l|--log <file>] [--keep-up] [--version <tag>] [-- jest args]
 # Examples:
-#   scripts/run-integration-oss.sh                       # full OSS-gated suite
+#   scripts/run-integration-oss.sh                       # full OSS-gated suite, default image tag
 #   scripts/run-integration-oss.sh --test WorkflowExecutor
 #   scripts/run-integration-oss.sh --log /tmp/oss.log    # custom log path
 #   scripts/run-integration-oss.sh --keep-up             # leave the stack running afterwards
+#   scripts/run-integration-oss.sh --version 3.33.0-rc1  # override the OSS image tag
 #   scripts/run-integration-oss.sh -- --testPathPatterns="EventClient"
 set -euo pipefail
 
@@ -35,12 +36,22 @@ while [[ $# -gt 0 ]]; do
     -t|--test) TEST_PATTERN="${2:?--test needs a path or pattern}"; shift 2 ;;
     -l|--log)  LOG_FILE="${2:?--log needs a file path}"; shift 2 ;;
     --keep-up) KEEP_UP=1; shift ;;
+    --version) OSS_CONDUCTOR_VERSION="${2:?--version needs a tag}"; shift 2 ;;
     -h|--help) usage; exit 0 ;;
     --)        shift; extra=("$@"); break ;;
     *) echo "Unknown argument: $1" >&2; usage; exit 1 ;;
   esac
 done
 
+# No default is applied here on purpose. The default tag is written once, in the
+# `image:` line of scripts/docker-compose-oss.yaml, so leaving OSS_CONDUCTOR_VERSION
+# unset lets compose supply it -- the same path a fork PR takes in CI. Only export
+# it when the caller actually asked for a specific tag, otherwise a value set but
+# not exported in the caller's shell would never reach compose anyway.
+if [[ -n "${OSS_CONDUCTOR_VERSION:-}" ]]; then
+  export OSS_CONDUCTOR_VERSION
+fi
+
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)"
 COMPOSE_FILE="${SCRIPT_DIR}/docker-compose-oss.yaml"
@@ -76,6 +87,11 @@ HEALTH_URL="${CONDUCTOR_SERVER_URL%/api}/health"
 compose() { docker compose -f "${COMPOSE_FILE}" "$@"; }
 
 cleanup() {
+  local status=$?
+  if [[ "${status}" -ne 0 ]]; then
+    echo "Dumping conductor-server logs (exit ${status})..." >&2
+    compose logs conductor-server || true
+  fi
   if [[ "${KEEP_UP}" == "1" ]]; then
     echo "--keep-up set: leaving the OSS stack running. Tear down with:"
     echo "  docker compose -f ${COMPOSE_FILE} down -v"
@@ -86,6 +102,18 @@ cleanup() {
 }
 trap cleanup EXIT
 
+# Ask compose what it resolved rather than reconstructing the tag here, so this
+# stays correct whether the tag came from --version or from the compose default.
+SERVER_IMAGE="$(compose config --images conductor-server | head -1)"
+echo "Using ${SERVER_IMAGE}"
+
+# `docker compose up` only pulls an image when it is missing locally, so a
+# previously-cached mutable tag (a re-pushed rc, or `latest` if that is what was
+# asked for) would silently be reused instead of getting the current version.
+# Pull unconditionally so the stack always reflects the tag we just printed.
+echo "Pulling ${SERVER_IMAGE} to ensure it's current..."
+compose pull conductor-server
+
 echo "Starting Conductor OSS stack (${COMPOSE_FILE})..."
 compose up -d
 
@@ -96,7 +124,6 @@ deadline=$(( SECONDS + HEALTH_TIMEOUT ))
 until curl -sf "${HEALTH_URL}" >/dev/null 2>&1; do
   if (( SECONDS >= deadline )); then
     echo "Error: Conductor did not become healthy within ${HEALTH_TIMEOUT}s." >&2
-    compose logs conductor-server || true
     exit 1
   fi
   sleep 5

src/integration-tests/WorkflowExecutor.test.ts

@@ -34,6 +34,7 @@ import {
   describeForOrkesV5,
   describeForOrkesOnlyV4,
   describeForOrkesOnlyV5,
+  testForRegionDurable,
 } from "./utils/customJestDescribe";
 import { registerWorkflowDefWithRetry, registerWorkflowWithRetry } from "./utils/registerWorkflowWithRetry";
 import { HTTPBIN_BASE_URL } from "./utils/testConstants";
@@ -427,7 +428,18 @@ describe("WorkflowExecutor", () => {
 
     describe("Execute Workflow with Return Strategies and Consistency Levels", () => {
       // Test data for combinations
-      const testCombinations = [
+      interface ConsistencyTestCase {
+        name: string;
+        consistency: Consistency;
+        returnStrategy: ReturnStrategy;
+        shouldHaveWorkflowFields: boolean;
+        shouldHaveTaskFields: boolean;
+        // Needs a server with cross-region replication configured; see
+        // testForRegionDurable.
+        requiresRegionDurable?: boolean;
+      }
+
+      const testCombinations: ConsistencyTestCase[] = [
         // SYNCHRONOUS consistency tests
         {
           name: "SYNC + TARGET_WORKFLOW",
@@ -457,34 +469,75 @@ describe("WorkflowExecutor", () => {
           shouldHaveWorkflowFields: false,
           shouldHaveTaskFields: true,
         },
-        // REGION_DURABLE consistency tests
+        // DURABLE consistency tests. DURABLE is the default consistency level and
+        // takes a different executor path than SYNCHRONOUS (async decider queue via
+        // scheduleWorkflow, rather than an inline decide), so the return strategies
+        // are worth covering against it too. These cases are what the REGION_DURABLE
+        // block below was really exercising before orkes-conductor 5.5.0, when
+        // REGION_DURABLE was accepted and silently treated as a plain local start.
         {
           name: "DURABLE + TARGET_WORKFLOW",
-          consistency: Consistency.REGION_DURABLE,
+          consistency: Consistency.DURABLE,
           returnStrategy: ReturnStrategy.TARGET_WORKFLOW,
           shouldHaveWorkflowFields: true,
           shouldHaveTaskFields: false,
         },
         {
           name: "DURABLE + BLOCKING_WORKFLOW",
-          consistency: Consistency.REGION_DURABLE,
+          consistency: Consistency.DURABLE,
           returnStrategy: ReturnStrategy.BLOCKING_WORKFLOW,
           shouldHaveWorkflowFields: true,
           shouldHaveTaskFields: false,
         },
         {
           name: "DURABLE + BLOCKING_TASK",
-          consistency: Consistency.REGION_DURABLE,
+          consistency: Consistency.DURABLE,
           returnStrategy: ReturnStrategy.BLOCKING_TASK,
           shouldHaveWorkflowFields: false,
           shouldHaveTaskFields: true,
         },
         {
           name: "DURABLE + BLOCKING_TASK_INPUT",
+          consistency: Consistency.DURABLE,
+          returnStrategy: ReturnStrategy.BLOCKING_TASK_INPUT,
+          shouldHaveWorkflowFields: false,
+          shouldHaveTaskFields: true,
+        },
+        // REGION_DURABLE consistency tests. Gated on CONDUCTOR_REGION_DURABLE_ENABLED
+        // — see testForRegionDurable. These were previously named "DURABLE + *" while
+        // requesting REGION_DURABLE, which is why they only started failing when
+        // sdkdev moved to a server build that honours the flag.
+        {
+          name: "REGION_DURABLE + TARGET_WORKFLOW",
+          consistency: Consistency.REGION_DURABLE,
+          returnStrategy: ReturnStrategy.TARGET_WORKFLOW,
+          shouldHaveWorkflowFields: true,
+          shouldHaveTaskFields: false,
+          requiresRegionDurable: true,
+        },
+        {
+          name: "REGION_DURABLE + BLOCKING_WORKFLOW",
+          consistency: Consistency.REGION_DURABLE,
+          returnStrategy: ReturnStrategy.BLOCKING_WORKFLOW,
+          shouldHaveWorkflowFields: true,
+          shouldHaveTaskFields: false,
+          requiresRegionDurable: true,
+        },
+        {
+          name: "REGION_DURABLE + BLOCKING_TASK",
+          consistency: Consistency.REGION_DURABLE,
+          returnStrategy: ReturnStrategy.BLOCKING_TASK,
+          shouldHaveWorkflowFields: false,
+          shouldHaveTaskFields: true,
+          requiresRegionDurable: true,
+        },
+        {
+          name: "REGION_DURABLE + BLOCKING_TASK_INPUT",
           consistency: Consistency.REGION_DURABLE,
           returnStrategy: ReturnStrategy.BLOCKING_TASK_INPUT,
           shouldHaveWorkflowFields: false,
           shouldHaveTaskFields: true,
+          requiresRegionDurable: true,
         },
       ];
 
@@ -660,7 +713,10 @@ describe("WorkflowExecutor", () => {
 
       // Now replicate for all other combinations
       testCombinations.slice(1).forEach((testCase) => {
-        test(`Should execute complex workflow with ${testCase.name}`, async () => {
+        const testFn = testCase.requiresRegionDurable
+          ? testForRegionDurable
+          : test;
+        testFn(`Should execute complex workflow with ${testCase.name}`, async () => {
           console.log(`\n--- Testing ${testCase.name} ---`);
 
           // Execute workflow

src/integration-tests/utils/customJestDescribe.ts

@@ -1,4 +1,4 @@
-import { describe } from "@jest/globals";
+import { describe, test } from "@jest/globals";
 
 const orkesBackendVersion = Number(process.env.ORKES_BACKEND_VERSION);
 const isOss = (process.env.CONDUCTOR_SERVER_TYPE || "").toLowerCase() === "oss";
@@ -31,3 +31,25 @@ export const describeForOrkesOnlyV5 =
 // skip it there for now
 export const describeForOssSchedulerWip =
   !ossGated && orkesBackendVersion >= 4 ? describe : describe.skip;
+
+// Consistency.REGION_DURABLE requires the target server to have cross-region
+// replication configured. Since orkes-conductor 5.5.0, a node without it rejects the
+// start outright ("REGION_DURABLE consistency requested but region replication is not
+// enabled/configured on this node") rather than silently downgrading to a plain local
+// start — it refuses a guarantee it cannot provide instead of appearing to honour it.
+// So these run only where the capability actually exists: set
+// CONDUCTOR_REGION_DURABLE_ENABLED=true against such a target.
+//
+// Deliberately an explicit opt-in rather than catching the 500 and skipping: auto-skip
+// would restore exactly the silent green that hid this for months (pre-5.5.0 the flag
+// was accepted and ignored, so these cases were really exercising DURABLE), and would
+// hide a genuine replication regression on a cluster where it is supposed to work.
+// Mirrors the java-sdk gate of the same name (TaskClientTests, PR #167).
+//
+// Test-level rather than describe-level because the cases are generated from a
+// combination table; it lives here so all suite gating stays in one module.
+const regionDurableEnabled = ["1", "true"].includes(
+  (process.env.CONDUCTOR_REGION_DURABLE_ENABLED || "").toLowerCase()
+);
+
+export const testForRegionDurable = regionDurableEnabled ? test : test.skip;