From 4c03fe54b0564d9011f0f26753ea87b6c02da9b0 Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Wed, 9 Sep 2026 12:50:43 -0600 Subject: [PATCH 1/3] test(integration): gate REGION_DURABLE cases, cover DURABLE, pin OSS image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four "DURABLE + *" cases in WorkflowExecutor.test.ts have been failing on sdkdev across Node 20/22/24 (shard 1/3) with REGION_DURABLE consistency requested but region replication is not enabled/configured on this node Nothing changed in this repo: the cases were named "DURABLE + *" but had always passed Consistency.REGION_DURABLE. They went red when sdkdev picked up a server build carrying the active/active work (orkes-conductor ea0a37ad74, "region-durable workflow start"). Before it, REGION_DURABLE was an inert enum value that fell through the same scheduleWorkflow branch as DURABLE, so the flag was accepted and silently ignored; after it, a node with no replicator wired rejects the start outright rather than appearing to honour a guarantee it cannot provide. The server is right — sdkdev is a single-node instance not meant to have replication — and the tests were green under false pretences: they asked for cross-region durability, got a plain local start, and asserted success. So: - Rename the four cases to "REGION_DURABLE + *", matching what they request, and gate them on CONDUCTOR_REGION_DURABLE_ENABLED via a new testForRegionDurable helper. Explicit opt-in rather than catching the 500 and skipping: auto-skip would restore exactly the silent green that hid this for months, and would mask a genuine replication regression on a cluster where it is meant to work. Same gate name as the java-sdk fix (orkes-io/java-sdk#167) so both SDKs configure identically. - Add four real "DURABLE + *" cases. Consistency.DURABLE appeared nowhere in the suite despite being the default level, and it takes a different executor path than SYNCHRONOUS (async decider queue via scheduleWorkflow, not an inline decide), so the return strategies are worth covering against it. This is not new coverage so much as restored: it is what the REGION_DURABLE cases were really exercising before the server started honouring the flag. Also pins the OSS Conductor image instead of tracking `latest`, so an upstream release cannot turn the OSS job red on an unrelated PR — the same class of surprise as the sdkdev upgrade above. The tag comes from the E2E_TEST_OSS_CONDUCTOR_VERSION org variable or a workflow_dispatch input, with a preflight step that fails with an actionable message when neither resolves, and scripts/run-integration-oss.sh grows a --version flag (defaulting to `latest`) for local runs. Net effect on CI: sdkdev goes from 4 failures to 4 skips plus 4 new passing cases. No CI configuration needs the new variable — gated-off is correct for a single-region server, and java-sdk likewise sets it in no workflow. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pull_request.yml | 13 ++++ scripts/docker-compose-oss.yaml | 5 +- scripts/run-integration-oss.sh | 8 ++- .../WorkflowExecutor.test.ts | 68 +++++++++++++++++-- .../utils/customJestDescribe.ts | 24 ++++++- 5 files changed, 108 insertions(+), 10 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index a102c49f..2617a9cb 100644 --- a/.github/workflows/pull_request.yml +++ b/.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)' + required: false + type: string concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -216,7 +221,15 @@ jobs: CONDUCTOR_REQUEST_TIMEOUT_MS: "300000" CONDUCTOR_RETRY_SERVER_ERRORS: "true" HTTPBIN_SERVICE_HOSTNAME: httpbin + OSS_CONDUCTOR_VERSION: ${{ inputs.oss_conductor_version || vars.E2E_TEST_OSS_CONDUCTOR_VERSION }} steps: + - name: Verify OSS Conductor version is set + run: | + if [ -z "$OSS_CONDUCTOR_VERSION" ]; then + 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 + echo "Using conductoross/conductor:$OSS_CONDUCTOR_VERSION" - name: Checkout uses: actions/checkout@v4 - name: Set up Node diff --git a/scripts/docker-compose-oss.yaml b/scripts/docker-compose-oss.yaml index 012a750f..0f7911e2 100644 --- a/scripts/docker-compose-oss.yaml +++ b/scripts/docker-compose-oss.yaml @@ -4,9 +4,12 @@ # # The Conductor server reaches httpbin over the compose network at # http://httpbin:8081, which matches HTTPBIN_SERVICE_HOSTNAME=httpbin. +# +# OSS_CONDUCTOR_VERSION defaults to `latest` for local runs; CI pins it via the +# E2E_TEST_OSS_CONDUCTOR_VERSION org variable (or a workflow_dispatch input). services: conductor-server: - image: conductoross/conductor:latest + image: conductoross/conductor:${OSS_CONDUCTOR_VERSION:-latest} environment: - CONFIG_PROP=config-postgres.properties ports: diff --git a/scripts/run-integration-oss.sh b/scripts/run-integration-oss.sh index 54945727..2240db15 100755 --- a/scripts/run-integration-oss.sh +++ b/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 ] [-l|--log ] [--keep-up] [-- jest args] +# scripts/run-integration-oss.sh [-t|--test ] [-l|--log ] [--keep-up] [--version ] [-- jest args] # Examples: -# scripts/run-integration-oss.sh # full OSS-gated suite +# scripts/run-integration-oss.sh # full OSS-gated suite, image `latest` # 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.32.0-rc18 # pin a specific OSS image tag # scripts/run-integration-oss.sh -- --testPathPatterns="EventClient" set -euo pipefail @@ -35,12 +36,15 @@ 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 +export OSS_CONDUCTOR_VERSION="${OSS_CONDUCTOR_VERSION:-latest}" + SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" COMPOSE_FILE="${SCRIPT_DIR}/docker-compose-oss.yaml" diff --git a/src/integration-tests/WorkflowExecutor.test.ts b/src/integration-tests/WorkflowExecutor.test.ts index d0e4f2ff..1f8bc8de 100644 --- a/src/integration-tests/WorkflowExecutor.test.ts +++ b/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 diff --git a/src/integration-tests/utils/customJestDescribe.ts b/src/integration-tests/utils/customJestDescribe.ts index 0cf353fc..7385fa5e 100644 --- a/src/integration-tests/utils/customJestDescribe.ts +++ b/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; From 7d73c6b4e215ca34b558e2ea16afb42ef6e6216c Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Wed, 9 Sep 2026 13:18:34 -0600 Subject: [PATCH 2/3] ci(oss): keep fork PRs running, dump server logs, pull the pinned image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps against the OSS harness the other six SDKs share (ruby, python, go, csharp, java, rust), which this repo's variant was derived from without carrying across. Each change below is byte-identical to those, so the seven stay diffable. Fork PRs. GitHub withholds org/repo variables from pull_request runs on forks exactly as it withholds secrets, so vars.E2E_TEST_OSS_CONDUCTOR_VERSION is always "" for an outside contributor (observed in csharp-sdk#178). Resolving in the job `env` and null-checking therefore failed every fork PR with a config error the contributor has no power to fix — on a job that needs no secrets at all, only a tag, since it runs a local docker-compose stack against localhost. Resolve in a step writing $GITHUB_ENV instead, so the two ways the tag can come back empty get different treatment: a fork PR pins FORK_PR_FALLBACK_VERSION and keeps running, anything else still fails loudly rather than silently drifting onto the pin. The pin is 3.32.3, matching all six. Server logs on failure. cleanup() ran `compose down -v` without dumping anything, and the only `compose logs` call sat inside the health-wait loop — so logs were captured when the server never came up, but lost whenever the server started fine and the tests then failed. `-v` drops the volumes too, so there was no recovering them afterwards. Local runs were thus strictly worse than CI, which already dumps via an `if: failure()` step. Dump from the EXIT trap on any nonzero status, before the teardown and before the --keep-up early return. `local status=$?` is first so it reflects the failing command; `set -euo pipefail` (already present) carries a failing jest run through the tee pipeline so the trap sees it. The health-loop dump is removed as now redundant — its `exit 1` reaches the same trap — which also brings this script to zero in-loop dumps like the others. Image freshness. `docker compose up` only pulls when the image is missing locally, so a cached mutable tag — `latest`, or a re-pushed rc — was silently reused and nothing reported which image actually ran. Pinning the tag does not help if a stale layer is already cached under that name. Pull conductor-server unconditionally and echo the resolved tag first. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pull_request.yml | 31 +++++++++++++++++++++++++----- scripts/run-integration-oss.sh | 15 ++++++++++++++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 2617a9cb..cc0c6175 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -8,7 +8,7 @@ on: workflow_dispatch: inputs: oss_conductor_version: - description: 'OSS Conductor image tag (falls back to E2E_TEST_OSS_CONDUCTOR_VERSION org var)' + 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 @@ -221,15 +221,36 @@ jobs: CONDUCTOR_REQUEST_TIMEOUT_MS: "300000" CONDUCTOR_RETRY_SERVER_ERRORS: "true" HTTPBIN_SERVICE_HOSTNAME: httpbin - OSS_CONDUCTOR_VERSION: ${{ inputs.oss_conductor_version || vars.E2E_TEST_OSS_CONDUCTOR_VERSION }} + # Used only when the org variable is unreachable because the run is a fork + # PR -- see the resolve step below. + FORK_PR_FALLBACK_VERSION: '3.32.3' steps: - - name: Verify OSS Conductor version is set + # 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 pin one and keep running. + # - 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 pin. + - 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 [ -z "$OSS_CONDUCTOR_VERSION" ]; then + if [ -n "$REQUESTED_VERSION" ]; then + resolved="$REQUESTED_VERSION" + elif [ "$IS_FORK_PR" = "true" ]; then + resolved="$FORK_PR_FALLBACK_VERSION" + echo "::notice::Fork PR: org variables are withheld, pinning conductoross/conductor:${resolved}" + 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 - echo "Using conductoross/conductor:$OSS_CONDUCTOR_VERSION" + echo "OSS_CONDUCTOR_VERSION=${resolved}" >> "$GITHUB_ENV" + echo "Using conductoross/conductor:${resolved}" - name: Checkout uses: actions/checkout@v4 - name: Set up Node diff --git a/scripts/run-integration-oss.sh b/scripts/run-integration-oss.sh index 2240db15..37cde7f0 100755 --- a/scripts/run-integration-oss.sh +++ b/scripts/run-integration-oss.sh @@ -80,6 +80,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" @@ -90,6 +95,15 @@ cleanup() { } trap cleanup EXIT +echo "Using conductoross/conductor:${OSS_CONDUCTOR_VERSION}" + +# `docker compose up` only pulls an image when it is missing locally, so a +# previously-cached `latest` (or any other mutable tag, including a re-pushed +# rc) would silently be reused instead of getting the current version. Pull +# unconditionally so the stack always reflects the tag we just printed. +echo "Pulling conductoross/conductor:${OSS_CONDUCTOR_VERSION} to ensure it's current..." +compose pull conductor-server + echo "Starting Conductor OSS stack (${COMPOSE_FILE})..." compose up -d @@ -100,7 +114,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 From 522705108e4fdcb6369990235ee877c3d09d3bcb Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Thu, 10 Sep 2026 11:21:33 -0600 Subject: [PATCH 3/3] ci(oss): write the image tag once, pull it in CI, explain agent-e2e's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #176. Three separate points, all about the Conductor OSS image tag having more than one home. One tag, one home. The tag was written twice: FORK_PR_FALLBACK_VERSION in the integration-tests-oss job, and `latest` as the script's default. A local run therefore could not reproduce a CI failure, which is the whole point of the script existing. Rather than teach one of them to read the other, make the `image:` line of docker-compose-oss.yaml the only place it is written and let both fall through to it: the script no longer applies a default at all and exports OSS_CONDUCTOR_VERSION only when --version actually supplied one, and the fork-PR branch of the resolve step now leaves the variable unset instead of pinning its own copy. Fork CI and a plain local run land on the identical image by taking the identical path, with no YAML parsing on either side and one hardcode removed rather than a mechanism added. The non-fork empty case still fails loudly. The script's "Using ..." and "Pulling ..." lines now come from `compose config --images conductor-server` instead of reconstructing the tag, so they stay honest whether it came from the flag or the compose default. Worth stating plainly: E2E_TEST_OSS_CONDUCTOR_VERSION is currently set to `latest` at the org level, so the pin this series added is nominal until someone sets it to a real version. The compose default is what actually holds the line today, on fork PRs and locally. Pull in CI. The script pulls before `up`, CI did not. On a GitHub-hosted runner the VM is ephemeral and starts with no cached copy of this image, so `up` pulls anyway and the step is redundant today — kept regardless, because it costs no extra network pull (`up` then finds the image locally), it splits "couldn't pull the image" from "the stack didn't come up" into two distinct red steps, and it is what stops a mutable tag going stale the day this job moves to a self-hosted runner with a warm Docker daemon. It also prints the tag in use, which matters now that a fork PR's tag is not spelled out in the workflow. agent-e2e's near-namesake. CONDUCTOR_OSS_VERSION there reads like it should be the same knob and is not, so say why at the definition rather than leave the next reader to work it out. That one is a Maven Central artifact version for the conductor-server boot JAR; this one is a Docker Hub image tag. Different registries publish different version sets — Maven has no `latest`, which is exactly what the org variable holds — and the agent suites additionally need >= 3.32.0-rc.8 for the /agent/* control plane and TaskDef.runtimeMetadata persistence they exercise. Pinned independently, deliberately. Converging the two is possible once the org variable holds a concrete version (3.32.3 and 3.33.0-rc1 are published to both registries), but that is a bump of a stale rc18 and a rename, not this change. This does move the OSS harness off byte-identical with the other six SDKs, which the previous commit had just restored. Deliberate and temporary: the same edit applies verbatim to java-sdk and python-sdk, held back for now. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/agent-e2e.yml | 13 +++++++++++- .github/workflows/pull_request.yml | 32 ++++++++++++++++++++---------- scripts/docker-compose-oss.yaml | 10 +++++++--- scripts/run-integration-oss.sh | 26 ++++++++++++++++-------- 4 files changed, 59 insertions(+), 22 deletions(-) diff --git a/.github/workflows/agent-e2e.yml b/.github/workflows/agent-e2e.yml index 6202f76e..158a027a 100644 --- a/.github/workflows/agent-e2e.yml +++ b/.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: diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index cc0c6175..e1aca655 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -221,9 +221,6 @@ jobs: CONDUCTOR_REQUEST_TIMEOUT_MS: "300000" CONDUCTOR_RETRY_SERVER_ERRORS: "true" HTTPBIN_SERVICE_HOSTNAME: httpbin - # Used only when the org variable is unreachable because the run is a fork - # PR -- see the resolve step below. - FORK_PR_FALLBACK_VERSION: '3.32.3' 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: @@ -231,26 +228,27 @@ jobs: # - 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 pin one and keep running. + # 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 pin. + # 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 - resolved="$REQUESTED_VERSION" + echo "OSS_CONDUCTOR_VERSION=${REQUESTED_VERSION}" >> "$GITHUB_ENV" elif [ "$IS_FORK_PR" = "true" ]; then - resolved="$FORK_PR_FALLBACK_VERSION" - echo "::notice::Fork PR: org variables are withheld, pinning conductoross/conductor:${resolved}" + 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 - echo "OSS_CONDUCTOR_VERSION=${resolved}" >> "$GITHUB_ENV" - echo "Using conductoross/conductor:${resolved}" - name: Checkout uses: actions/checkout@v4 - name: Set up Node @@ -269,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 diff --git a/scripts/docker-compose-oss.yaml b/scripts/docker-compose-oss.yaml index 0f7911e2..58613c56 100644 --- a/scripts/docker-compose-oss.yaml +++ b/scripts/docker-compose-oss.yaml @@ -5,11 +5,15 @@ # The Conductor server reaches httpbin over the compose network at # http://httpbin:8081, which matches HTTPBIN_SERVICE_HOSTNAME=httpbin. # -# OSS_CONDUCTOR_VERSION defaults to `latest` for local runs; CI pins it via the -# E2E_TEST_OSS_CONDUCTOR_VERSION org variable (or a workflow_dispatch input). +# 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:${OSS_CONDUCTOR_VERSION:-latest} + image: conductoross/conductor:${OSS_CONDUCTOR_VERSION:-3.32.3} environment: - CONFIG_PROP=config-postgres.properties ports: diff --git a/scripts/run-integration-oss.sh b/scripts/run-integration-oss.sh index 37cde7f0..b5e4708e 100755 --- a/scripts/run-integration-oss.sh +++ b/scripts/run-integration-oss.sh @@ -14,11 +14,11 @@ # Usage: # scripts/run-integration-oss.sh [-t|--test ] [-l|--log ] [--keep-up] [--version ] [-- jest args] # Examples: -# scripts/run-integration-oss.sh # full OSS-gated suite, image `latest` +# 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.32.0-rc18 # pin a specific OSS image tag +# 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 @@ -43,7 +43,14 @@ while [[ $# -gt 0 ]]; do esac done -export OSS_CONDUCTOR_VERSION="${OSS_CONDUCTOR_VERSION:-latest}" +# 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)" @@ -95,13 +102,16 @@ cleanup() { } trap cleanup EXIT -echo "Using conductoross/conductor:${OSS_CONDUCTOR_VERSION}" +# 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 `latest` (or any other mutable tag, including a re-pushed -# rc) would silently be reused instead of getting the current version. Pull -# unconditionally so the stack always reflects the tag we just printed. -echo "Pulling conductoross/conductor:${OSS_CONDUCTOR_VERSION} to ensure it's current..." +# 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})..."