From a5a352895b45d0bc0d96619636528dff48783964 Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Thu, 10 Sep 2026 12:24:26 -0600 Subject: [PATCH 1/4] - Improve OSS Conductor E2E setup and local run tooling, including compose isolation and server-log capture on failures. - Make E2E task registration/completion safe for concurrent PR and test runs. - Fix OSS Conductor version selection in CI with a fallback for fork PRs that cannot access org variables. - Remove outdated gating from TaskUpdateV2Tests and clean up related documentation/comments. --- .github/workflows/pull_request.yml | 74 ++++++------ .../Environment/EnvironmentVariableTests.cs | 4 + Tests/Integration/Helpers/TestPrefix.cs | 19 ++- Tests/Integration/Task/TaskPollTests.cs | 5 +- Tests/Integration/Task/TaskUpdateTests.cs | 77 +++++++++---- Tests/Integration/V5/TaskUpdateV2Tests.cs | 108 +++++++++++------- .../Workflow/WorkflowLifecycleTests.cs | 3 + docs/workflow-testing.md | 27 +++++ scripts/docker-compose-oss.yaml | 36 ++++++ scripts/run-integration-oss.sh | 89 +++++++++++++++ 10 files changed, 332 insertions(+), 110 deletions(-) create mode 100644 scripts/docker-compose-oss.yaml create mode 100755 scripts/run-integration-oss.sh diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 14cb31d6..7694bf42 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -14,7 +14,7 @@ on: 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)' + 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 @@ -173,50 +173,40 @@ jobs: runs-on: ubuntu-latest env: CONDUCTOR_SERVER_URL: http://localhost:8080/api - # The literal fallback is not redundant: GitHub withholds org/repo - # variables from pull_request runs on forks exactly as it withholds - # secrets, so vars.* resolves to "" there. This job needs no secrets — - # only a tag — so it pins one and keeps running on fork PRs instead of - # failing with nothing to pull. - OSS_CONDUCTOR_VERSION: ${{ inputs.oss_conductor_version || vars.E2E_TEST_OSS_CONDUCTOR_VERSION || '3.32.3' }} + # 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: Show OSS Conductor version - run: echo "Using conductoross/conductor:$OSS_CONDUCTOR_VERSION" + # 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 [ -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 "OSS_CONDUCTOR_VERSION=${resolved}" >> "$GITHUB_ENV" + echo "Using conductoross/conductor:${resolved}" - name: Checkout uses: actions/checkout@v4 - - name: Write docker-compose file - run: | - cat <<'EOF' > docker-compose-oss.yaml - services: - conductor-server: - image: conductoross/conductor:${{ env.OSS_CONDUCTOR_VERSION }} - environment: - - CONFIG_PROP=config-postgres.properties - ports: - - "8080:8080" - healthcheck: - test: ["CMD", "curl", "-I", "-XGET", "http://localhost:8080/health"] - interval: 10s - timeout: 10s - retries: 20 - links: - - conductor-postgres:postgresdb - depends_on: - conductor-postgres: - condition: service_healthy - conductor-postgres: - image: postgres:16 - environment: - - POSTGRES_USER=conductor - - POSTGRES_PASSWORD=conductor - healthcheck: - test: timeout 5 bash -c 'cat < /dev/null > /dev/tcp/localhost/5432' - interval: 5s - timeout: 5s - retries: 12 - EOF - name: Start Conductor OSS stack - run: docker compose -f docker-compose-oss.yaml up -d + run: docker compose -f scripts/docker-compose-oss.yaml up -d - name: Wait for Conductor to be healthy run: timeout 120 bash -c 'until curl -sf http://localhost:8080/health; do sleep 5; done' - name: Setup .NET @@ -231,4 +221,4 @@ jobs: -l "console;verbosity=normal" - name: Dump Conductor logs if: failure() - run: docker compose -f docker-compose-oss.yaml logs conductor-server + run: docker compose -f scripts/docker-compose-oss.yaml logs conductor-server diff --git a/Tests/Integration/Environment/EnvironmentVariableTests.cs b/Tests/Integration/Environment/EnvironmentVariableTests.cs index 350c1e62..c742cfb2 100644 --- a/Tests/Integration/Environment/EnvironmentVariableTests.cs +++ b/Tests/Integration/Environment/EnvironmentVariableTests.cs @@ -18,6 +18,10 @@ namespace Tests.Integration.Environment { [Collection("Integration")] [Trait("Category", "Integration")] + // Orkes-only: OSS exposes the environment API read-only. Its EnvironmentResource declares + // only GET /api/environment and GET /api/environment/{key} (added by + // conductor-oss/conductor#1251, first released in 3.32.0-rc.5 and 3.31.2), so the + // CreateOrUpdateEnvVariable calls below (PUT /environment/{key}) return 405 against OSS. [Trait("ServerType", "Orkes")] public class EnvironmentVariableTests : IClassFixture { diff --git a/Tests/Integration/Helpers/TestPrefix.cs b/Tests/Integration/Helpers/TestPrefix.cs index 8a9b6014..482f61c3 100644 --- a/Tests/Integration/Helpers/TestPrefix.cs +++ b/Tests/Integration/Helpers/TestPrefix.cs @@ -17,15 +17,28 @@ namespace Tests.Integration.Helpers /// /// Generates unique resource names per test run to avoid conflicts /// between concurrent runs or leftover data from previous runs. - /// Format: csharp_sdk_{shortRunId}_{name} + /// Format: csharp_sdk_{shortRunId}[_{attempt}]_{name} /// public static class TestPrefix { - private static readonly string RunId = SysEnv.GetEnvironmentVariable("GITHUB_RUN_ID") - ?? System.Guid.NewGuid().ToString("N")[..8]; + private static readonly string RunId = ResolveRunId(); public static string Prefix => $"csharp_sdk_{RunId}"; public static string Name(string name) => $"{Prefix}_{name}"; + + // GITHUB_RUN_ID is stable across attempts of the same run, so on a re-run the task + // queues would still be named after the failed attempt — and a task it orphaned (e.g. + // a StartWorkflow whose response was lost) is still sitting in one, ready to be polled + // by a test that did not create it. Including the attempt gives each try its own names. + private static string ResolveRunId() + { + var runId = SysEnv.GetEnvironmentVariable("GITHUB_RUN_ID"); + if (string.IsNullOrEmpty(runId)) + return System.Guid.NewGuid().ToString("N")[..8]; + + var attempt = SysEnv.GetEnvironmentVariable("GITHUB_RUN_ATTEMPT"); + return string.IsNullOrEmpty(attempt) ? runId : $"{runId}_{attempt}"; + } } } diff --git a/Tests/Integration/Task/TaskPollTests.cs b/Tests/Integration/Task/TaskPollTests.cs index 7ffe7cdd..eaecce20 100644 --- a/Tests/Integration/Task/TaskPollTests.cs +++ b/Tests/Integration/Task/TaskPollTests.cs @@ -95,9 +95,12 @@ private void Cleanup(string id) { try { + // Report each task against the workflow it actually belongs to — a batch poll by + // task type can return tasks from other executions, and passing `id` for those + // would complete them against the wrong workflow. var tasks = _taskClient.BatchPoll(_taskName, WorkerId, domain: null, count: 10); foreach (var t in tasks ?? new List()) - _taskClient.UpdateTask(new TaskResult { TaskId = t.TaskId, WorkflowInstanceId = id, Status = TaskResult.StatusEnum.COMPLETED }); + _taskClient.UpdateTask(new TaskResult { TaskId = t.TaskId, WorkflowInstanceId = t.WorkflowInstanceId, Status = TaskResult.StatusEnum.COMPLETED }); } catch { } try { _workflowClient.Terminate(id); } catch { } diff --git a/Tests/Integration/Task/TaskUpdateTests.cs b/Tests/Integration/Task/TaskUpdateTests.cs index a9463e0d..3876ad77 100644 --- a/Tests/Integration/Task/TaskUpdateTests.cs +++ b/Tests/Integration/Task/TaskUpdateTests.cs @@ -54,34 +54,48 @@ public TaskUpdateTests(ConductorFixture fixture) public void CompleteTask_WorkflowCompletes() { var id = StartWorkflow(); - var task = PollTask(id); - - _taskClient.UpdateTask(new TaskResult + try { - TaskId = task.TaskId, - WorkflowInstanceId = id, - Status = TaskResult.StatusEnum.COMPLETED, - OutputData = new Dictionary { { "result", "ok" } } - }); + var task = PollTask(id); - Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.COMPLETED, GetWorkflowStatus(id)); + _taskClient.UpdateTask(new TaskResult + { + TaskId = task.TaskId, + WorkflowInstanceId = id, + Status = TaskResult.StatusEnum.COMPLETED, + OutputData = new Dictionary { { "result", "ok" } } + }); + + Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.COMPLETED, GetWorkflowStatus(id)); + } + finally + { + Cleanup(id); + } } [Fact] public void FailTask_WorkflowFails() { var id = StartWorkflow(); - var task = PollTask(id); - - _taskClient.UpdateTask(new TaskResult + try { - TaskId = task.TaskId, - WorkflowInstanceId = id, - Status = TaskResult.StatusEnum.FAILED, - ReasonForIncompletion = "deliberate test failure" - }); + var task = PollTask(id); + + _taskClient.UpdateTask(new TaskResult + { + TaskId = task.TaskId, + WorkflowInstanceId = id, + Status = TaskResult.StatusEnum.FAILED, + ReasonForIncompletion = "deliberate test failure" + }); - Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.FAILED, GetWorkflowStatus(id)); + Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.FAILED, GetWorkflowStatus(id)); + } + finally + { + Cleanup(id); + } } [Fact] @@ -118,16 +132,26 @@ public void GetTask_ReturnsTaskDetails() private string StartWorkflow() => _workflowClient.StartWorkflow(new StartWorkflowRequest(name: _workflowName)); + // Polling is by task type, so the queue can hand back a task from another execution of + // the same definition. Writing our result to it would leave our workflow stuck and + // stamp this test's outcome on an unrelated one, so skip foreign tasks — leaving them + // claimed keeps them out of the queue — and keep polling for our own. private Conductor.Client.Models.Task PollTask(string workflowId) { - Conductor.Client.Models.Task task = null; - for (var i = 0; i < 10 && task == null; i++) + for (var i = 0; i < 20; i++) { - task = _taskClient.Poll(_taskName, WorkerId); - if (task == null) System.Threading.Thread.Sleep(500); + var task = _taskClient.Poll(_taskName, WorkerId); + if (task == null) + { + System.Threading.Thread.Sleep(500); + continue; + } + if (task.WorkflowInstanceId == workflowId) + return task; } - Assert.NotNull(task); - return task; + + Assert.True(false, $"No task for workflow {workflowId} appeared in queue {_taskName}"); + return null; } private Conductor.Client.Models.Workflow.StatusEnum? GetWorkflowStatus(string id) @@ -145,6 +169,11 @@ private Conductor.Client.Models.Task PollTask(string workflowId) private void Cleanup(string workflowId, string taskId) { try { _taskClient.UpdateTask(new TaskResult { TaskId = taskId, WorkflowInstanceId = workflowId, Status = TaskResult.StatusEnum.COMPLETED }); } catch { } + Cleanup(workflowId); + } + + private void Cleanup(string workflowId) + { try { _workflowClient.Terminate(workflowId); } catch { } try { _workflowClient.Delete(workflowId); } catch { } } diff --git a/Tests/Integration/V5/TaskUpdateV2Tests.cs b/Tests/Integration/V5/TaskUpdateV2Tests.cs index 7d017783..7016618c 100644 --- a/Tests/Integration/V5/TaskUpdateV2Tests.cs +++ b/Tests/Integration/V5/TaskUpdateV2Tests.cs @@ -23,12 +23,16 @@ namespace Tests.Integration.V5 { /// - /// Tests for task-update-v2 (PUT /tasks/{taskId}). - /// Runs only against v5 — excluded from v4 CI job via Version!=V5Only filter. + /// Task update via POST /tasks: polls a task, reports a terminal TaskResult, and checks that + /// both the task and its workflow reach the expected state. Runs against every server the + /// integration job targets — that endpoint is not version-specific. + /// + /// Despite the class name, this does not exercise task-update-v2 (POST /tasks/update-v2, + /// which returns the next available task); the SDK has no binding for that endpoint. + /// /// [Collection("Integration")] [Trait("Category", "Integration")] - [Trait("Version", "V5Only")] public class TaskUpdateV2Tests : IClassFixture { private readonly WorkflowResourceApi _workflowClient; @@ -64,25 +68,30 @@ public void UpdateTask_CompletesTask_WorkflowCompletes() { var workflowId = StartWorkflow(); _output.WriteLine($"WorkflowId: {workflowId}"); - - var task = PollTask(); - _output.WriteLine($"TaskId: {task.TaskId}, TaskType: {task.TaskDefName}, Status: {task.Status}"); - - var taskResult = new TaskResult + try { - TaskId = task.TaskId, - WorkflowInstanceId = workflowId, - Status = TaskResult.StatusEnum.COMPLETED, - OutputData = new Dictionary { { "result", "ok" } } - }; + var task = PollTask(workflowId); + _output.WriteLine($"TaskId: {task.TaskId}, TaskType: {task.TaskDefName}, Status: {task.Status}"); - var updateResponse = _taskClient.UpdateTask(taskResult); - _output.WriteLine($"UpdateTask response: {updateResponse}"); + var taskResult = new TaskResult + { + TaskId = task.TaskId, + WorkflowInstanceId = workflowId, + Status = TaskResult.StatusEnum.COMPLETED, + OutputData = new Dictionary { { "result", "ok" } } + }; - VerifyTaskUpdated(task.TaskId, taskResult); + var updateResponse = _taskClient.UpdateTask(taskResult); + _output.WriteLine($"UpdateTask response: {updateResponse}"); - Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.COMPLETED, GetWorkflowStatus(workflowId)); - Cleanup(workflowId); + VerifyTaskUpdated(task.TaskId, taskResult); + + Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.COMPLETED, GetWorkflowStatus(workflowId)); + } + finally + { + Cleanup(workflowId); + } } [Fact] @@ -90,25 +99,30 @@ public void UpdateTask_FailTask_WorkflowFails() { var workflowId = StartWorkflow(); _output.WriteLine($"WorkflowId: {workflowId}"); - - var task = PollTask(); - _output.WriteLine($"TaskId: {task.TaskId}, TaskType: {task.TaskDefName}, Status: {task.Status}"); - - var taskResult = new TaskResult + try { - TaskId = task.TaskId, - WorkflowInstanceId = workflowId, - Status = TaskResult.StatusEnum.FAILED, - ReasonForIncompletion = "v5 test failure" - }; + var task = PollTask(workflowId); + _output.WriteLine($"TaskId: {task.TaskId}, TaskType: {task.TaskDefName}, Status: {task.Status}"); + + var taskResult = new TaskResult + { + TaskId = task.TaskId, + WorkflowInstanceId = workflowId, + Status = TaskResult.StatusEnum.FAILED, + ReasonForIncompletion = "v5 test failure" + }; - var updateResponse = _taskClient.UpdateTask(taskResult); - _output.WriteLine($"UpdateTask response: {updateResponse}"); + var updateResponse = _taskClient.UpdateTask(taskResult); + _output.WriteLine($"UpdateTask response: {updateResponse}"); - VerifyTaskUpdated(task.TaskId, taskResult); + VerifyTaskUpdated(task.TaskId, taskResult); - Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.FAILED, GetWorkflowStatus(workflowId)); - Cleanup(workflowId); + Assert.Equal(Conductor.Client.Models.Workflow.StatusEnum.FAILED, GetWorkflowStatus(workflowId)); + } + finally + { + Cleanup(workflowId); + } } private void VerifyTaskUpdated(string taskId, TaskResult sentResult) @@ -133,16 +147,30 @@ private void VerifyTaskUpdated(string taskId, TaskResult sentResult) private string StartWorkflow() => _workflowClient.StartWorkflow(new StartWorkflowRequest(name: _workflowName)); - private Conductor.Client.Models.Task PollTask() + // Polls until a task belonging to `workflowId` is claimed. Polling is by task type, so + // the queue can hand back a task from another execution of the same definition — one + // orphaned by an earlier attempt, say. Reporting our result against that task would + // both leave our own workflow stuck and stamp this test's outcome onto an unrelated + // one, so foreign tasks are left claimed (which keeps them out of the queue) and + // never written to. + private Conductor.Client.Models.Task PollTask(string workflowId) { - Conductor.Client.Models.Task task = null; - for (var i = 0; i < 10 && task == null; i++) + for (var i = 0; i < 20; i++) { - task = _taskClient.Poll(_taskName, WorkerId); - if (task == null) System.Threading.Thread.Sleep(500); + var task = _taskClient.Poll(_taskName, WorkerId); + if (task == null) + { + System.Threading.Thread.Sleep(500); + continue; + } + if (task.WorkflowInstanceId == workflowId) + return task; + + _output.WriteLine($" Skipping task {task.TaskId} from workflow {task.WorkflowInstanceId}"); } - Assert.NotNull(task); - return task; + + Assert.True(false, $"No task for workflow {workflowId} appeared in queue {_taskName}"); + return null; } private Conductor.Client.Models.Workflow.StatusEnum? GetWorkflowStatus(string id) diff --git a/Tests/Integration/Workflow/WorkflowLifecycleTests.cs b/Tests/Integration/Workflow/WorkflowLifecycleTests.cs index 6f0748a3..6d2d693d 100644 --- a/Tests/Integration/Workflow/WorkflowLifecycleTests.cs +++ b/Tests/Integration/Workflow/WorkflowLifecycleTests.cs @@ -133,6 +133,9 @@ public void GetExecutionStatus_ReturnsWorkflowDetails() } [Fact] + // Orkes-only: PUT /workflow/{workflowId}/variables is not a registered OSS route — no + // REST controller declares it — so this returns 404 there. Unrelated to the + // SET_VARIABLE task type, which OSS does support. [Trait("ServerType", "Orkes")] public void UpdateWorkflowVariables_VariablesAreReflected() { diff --git a/docs/workflow-testing.md b/docs/workflow-testing.md index 5956fffb..d96b77b5 100644 --- a/docs/workflow-testing.md +++ b/docs/workflow-testing.md @@ -71,6 +71,33 @@ export CONDUCTOR_SERVER_URL=http://localhost:8080/api dotnet test Tests/conductor-csharp.test.csproj ``` +### Running the OSS integration suite locally + +`scripts/run-integration-oss.sh` mirrors the `integration_tests_oss` job in +`pull_request.yml`: it starts a local Conductor OSS + Postgres stack (defined in +`scripts/docker-compose-oss.yaml`), waits for `/health`, runs the integration suite with +Orkes-only tests filtered out (`ServerType!=Orkes`), and tears the stack down on exit. + +```shell +scripts/run-integration-oss.sh # against `latest` +scripts/run-integration-oss.sh --version 3.32.0-rc18 +scripts/run-integration-oss.sh --keep-up # leave the stack running afterwards +``` + +The script always prints the resolved `conductoross/conductor` tag and pulls it before +starting the stack, since `latest` (the local default) is a mutable tag — without an +explicit pull, `docker compose up` would silently reuse a stale cached image instead of +fetching the current one. + +Tests tagged `[Trait("ServerType", "Orkes")]` are excluded from this run because they +exercise features OSS does not implement — everything in `Tests/Integration/Orkes/`, plus +`EnvironmentVariableTests` and +`WorkflowLifecycleTests.UpdateWorkflowVariables_VariablesAreReflected`. Why each one is +gated, and the OSS endpoint it needs, is recorded in a comment next to the trait in the test +file itself. Read that before adding or removing the trait, and confirm the change against a +freshly-pulled image — a test that fails against a stale local image may pass against +current OSS. + ## Agent E2E suites `Conductor.AI.E2eTests/` is organised by feature, one numbered suite per area — basic diff --git a/scripts/docker-compose-oss.yaml b/scripts/docker-compose-oss.yaml new file mode 100644 index 00000000..750e98f3 --- /dev/null +++ b/scripts/docker-compose-oss.yaml @@ -0,0 +1,36 @@ +# Conductor OSS stack used to run the SDK integration tests against open-source +# Conductor. Shared by scripts/run-integration-oss.sh and the +# integration_tests_oss job in .github/workflows/pull_request.yml. +# +# OSS_CONDUCTOR_VERSION selects the server tag — in CI from the org-wide +# E2E_TEST_OSS_CONDUCTOR_VERSION variable (or a workflow_dispatch input), `latest` locally. +# +# Per-repo name; unnamed, the project defaults to this file's dir (`scripts`) in every SDK repo and stacks collide. +name: csharp-sdk-oss-e2e +services: + conductor-server: + image: conductoross/conductor:${OSS_CONDUCTOR_VERSION:-latest} + environment: + - CONFIG_PROP=config-postgres.properties + ports: + - "8080:8080" + healthcheck: + test: ["CMD", "curl", "-I", "-XGET", "http://localhost:8080/health"] + interval: 10s + timeout: 10s + retries: 20 + links: + - conductor-postgres:postgresdb + depends_on: + conductor-postgres: + condition: service_healthy + conductor-postgres: + image: postgres:16 + environment: + - POSTGRES_USER=conductor + - POSTGRES_PASSWORD=conductor + healthcheck: + test: timeout 5 bash -c 'cat < /dev/null > /dev/tcp/localhost/5432' + interval: 5s + timeout: 5s + retries: 12 diff --git a/scripts/run-integration-oss.sh b/scripts/run-integration-oss.sh new file mode 100755 index 00000000..732d9540 --- /dev/null +++ b/scripts/run-integration-oss.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# +# Spin up a local Conductor OSS stack and run the SDK integration suite against +# it, mirroring the `integration_tests_oss` job in +# .github/workflows/pull_request.yml. Orkes-only tests are gated out via the +# `ServerType!=Orkes` test filter. +# +# The stack (Conductor OSS + Postgres) is defined in +# scripts/docker-compose-oss.yaml and is torn down automatically on exit. The +# image is always pulled before starting, since `latest` (the local default) +# is a mutable tag and a cached copy would otherwise go stale silently. +# +# Usage: +# scripts/run-integration-oss.sh [--keep-up] [--version ] +# Examples: +# scripts/run-integration-oss.sh # run against `latest` +# scripts/run-integration-oss.sh --version 3.32.0-rc18 +# scripts/run-integration-oss.sh --keep-up # leave the stack running afterwards +set -euo pipefail + +KEEP_UP=0 + +while [[ $# -gt 0 ]]; do + case "$1" in + --keep-up) KEEP_UP=1; shift ;; + --version) OSS_CONDUCTOR_VERSION="${2:?--version needs a tag}"; shift 2 ;; + -h|--help) + echo "Usage: $0 [--keep-up] [--version ]" + exit 0 + ;; + *) echo "Unknown argument: $1" >&2; 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" +cd "${REPO_ROOT}" + +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" + return + fi + echo "Tearing down Conductor OSS stack..." + compose down -v || true +} +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) 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 up -d + +echo "Waiting for Conductor to be healthy..." +HEALTH_TIMEOUT="${HEALTH_TIMEOUT:-180}" +deadline=$(( SECONDS + HEALTH_TIMEOUT )) +until curl -sf http://localhost:8080/health >/dev/null 2>&1; do + if (( SECONDS >= deadline )); then + echo "Error: Conductor did not become healthy within ${HEALTH_TIMEOUT}s." >&2 + exit 1 + fi + sleep 5 +done +echo "Conductor is up." + +export CONDUCTOR_SERVER_URL="http://localhost:8080/api" + +dotnet test Tests/conductor-csharp.test.csproj \ + -p:DefineConstants=EXCLUDE_EXAMPLE_WORKERS \ + --filter "Category=Integration&ServerType!=Orkes" \ + -l "console;verbosity=normal" From 9db9623f79bd9fdec14732bc6a959ea8b97959dc Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Thu, 10 Sep 2026 12:50:45 -0600 Subject: [PATCH 2/4] ci(oss): write the image tag once, pull it in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports javascript-sdk#176's resolution of the same review feedback, so the OSS harness stays diffable across the SDKs. One tag, one home. The tag was written twice: FORK_PR_FALLBACK_VERSION in the integration_tests_oss job, and `latest` as the local script's default. A local run therefore could not reproduce a CI failure, which is most of why the script exists. 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 applies no default of its own 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 reach the identical image by 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 whichever source supplied it. Worth stating plainly: E2E_TEST_OSS_CONDUCTOR_VERSION is currently set to `latest` at the org level, so the pin is nominal for normal CI runs 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, 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. It sits after Checkout because it needs the compose file, and before the stack starts; .NET setup deliberately stays after the stack is up, as before. Not touched: agent-e2e.yml's CONDUCTOR_SERVER_VERSION. That is a Maven Central artifact version for the conductor-server boot JAR, not a Docker Hub image tag, and it is out of scope here. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pull_request.yml | 34 ++++++++++++++++++++---------- scripts/docker-compose-oss.yaml | 11 +++++++--- scripts/run-integration-oss.sh | 34 ++++++++++++++++++++---------- 3 files changed, 54 insertions(+), 25 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 7694bf42..68894007 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -14,7 +14,7 @@ on: 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)' + description: 'OSS Conductor image tag (falls back to the E2E_TEST_OSS_CONDUCTOR_VERSION org var, then to the default in scripts/docker-compose-oss.yaml on fork PRs)' required: false type: string @@ -173,9 +173,6 @@ jobs: runs-on: ubuntu-latest env: CONDUCTOR_SERVER_URL: http://localhost:8080/api - # 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: @@ -183,28 +180,43 @@ 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 + # `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 750e98f3..ba6ad093 100644 --- a/scripts/docker-compose-oss.yaml +++ b/scripts/docker-compose-oss.yaml @@ -2,14 +2,19 @@ # Conductor. Shared by scripts/run-integration-oss.sh and the # integration_tests_oss job in .github/workflows/pull_request.yml. # -# OSS_CONDUCTOR_VERSION selects the server tag — in CI from the org-wide -# E2E_TEST_OSS_CONDUCTOR_VERSION variable (or a workflow_dispatch input), `latest` locally. +# 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 org-wide +# E2E_TEST_OSS_CONDUCTOR_VERSION variable or a workflow_dispatch input. Bump the +# tag here and both follow. # # Per-repo name; unnamed, the project defaults to this file's dir (`scripts`) in every SDK repo and stacks collide. name: csharp-sdk-oss-e2e 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 732d9540..e0dabfa8 100755 --- a/scripts/run-integration-oss.sh +++ b/scripts/run-integration-oss.sh @@ -6,15 +6,17 @@ # `ServerType!=Orkes` test filter. # # The stack (Conductor OSS + Postgres) is defined in -# scripts/docker-compose-oss.yaml and is torn down automatically on exit. The -# image is always pulled before starting, since `latest` (the local default) -# is a mutable tag and a cached copy would otherwise go stale silently. +# scripts/docker-compose-oss.yaml and is torn down automatically on exit. That +# file's `image:` line is also where the default tag lives -- this script +# applies no default of its own, so a plain run and a fork-PR CI run land on +# the identical image. The image is always pulled before starting, since a tag +# can be mutable and a cached copy would otherwise go stale silently. # # Usage: # scripts/run-integration-oss.sh [--keep-up] [--version ] # Examples: -# scripts/run-integration-oss.sh # run against `latest` -# scripts/run-integration-oss.sh --version 3.32.0-rc18 +# scripts/run-integration-oss.sh # default tag from the compose file +# scripts/run-integration-oss.sh --version 3.33.0-rc1 # scripts/run-integration-oss.sh --keep-up # leave the stack running afterwards set -euo pipefail @@ -32,7 +34,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)" @@ -57,13 +66,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) 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..." From f1f55e61826153b6f492d9c0ee1eff6ee77a3e3b Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Fri, 11 Sep 2026 10:15:38 -0600 Subject: [PATCH 3/4] fix(oss): report the server image by name, not by position The "Using ..." line printed `postgres:16`. `docker compose config --images` lists every service's image and does not reliably honour the service-name filter it accepts, so `--images conductor-server | head -1` took whichever image compose happened to emit first -- the postgres one. Observed in java-sdk against the same compose layout. Cosmetic only: `compose pull conductor-server` and `compose up` both address the service directly and were always correct, so the stack has been running the right image throughout; only the echo was wrong. Select by image name instead. That couples to `conductoross/conductor`, which the line being replaced already hardcoded, so nothing new is pinned down. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pull_request.yml | 2 +- scripts/run-integration-oss.sh | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 68894007..dd04438d 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -215,7 +215,7 @@ jobs: # 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)" + echo "Using $(docker compose -f scripts/docker-compose-oss.yaml config --images | grep -m1 '^conductoross/conductor:')" 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 diff --git a/scripts/run-integration-oss.sh b/scripts/run-integration-oss.sh index e0dabfa8..eba20dbc 100755 --- a/scripts/run-integration-oss.sh +++ b/scripts/run-integration-oss.sh @@ -68,7 +68,9 @@ 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)" +# `--images` lists every service's image and does not reliably honour a service +# filter, so select the server's by name rather than by position. +SERVER_IMAGE="$(compose config --images | grep -m1 '^conductoross/conductor:')" echo "Using ${SERVER_IMAGE}" # `docker compose up` only pulls an image when it is missing locally, so a From 01867ac99aaf4507cb8aea690a05df80482d68f3 Mon Sep 17 00:00:00 2001 From: Chris Hagglund Date: Tue, 22 Sep 2026 09:42:15 -0600 Subject: [PATCH 4/4] address feedback re: stale documentation --- docs/workflow-testing.md | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/workflow-testing.md b/docs/workflow-testing.md index d96b77b5..db613446 100644 --- a/docs/workflow-testing.md +++ b/docs/workflow-testing.md @@ -79,15 +79,21 @@ dotnet test Tests/conductor-csharp.test.csproj Orkes-only tests filtered out (`ServerType!=Orkes`), and tears the stack down on exit. ```shell -scripts/run-integration-oss.sh # against `latest` -scripts/run-integration-oss.sh --version 3.32.0-rc18 +scripts/run-integration-oss.sh # default tag from scripts/docker-compose-oss.yaml +scripts/run-integration-oss.sh --version 3.33.0-rc1 scripts/run-integration-oss.sh --keep-up # leave the stack running afterwards ``` +The default tag is written in exactly one place — the `image:` line of +`scripts/docker-compose-oss.yaml` — and that is what both a plain local run and a fork-PR CI +run land on. A non-fork CI run overrides it with the `E2E_TEST_OSS_CONDUCTOR_VERSION` org +variable (currently `latest`), so CI and a local run are not necessarily on the same image; +pass `--version` to reproduce a specific CI run. + The script always prints the resolved `conductoross/conductor` tag and pulls it before -starting the stack, since `latest` (the local default) is a mutable tag — without an -explicit pull, `docker compose up` would silently reuse a stale cached image instead of -fetching the current one. +starting the stack, because none of the tags in play are immutable — `latest` plainly, and +rc tags get re-pushed. Without an explicit pull, `docker compose up` would silently reuse a +stale cached image instead of fetching the current one. Tests tagged `[Trait("ServerType", "Orkes")]` are excluded from this run because they exercise features OSS does not implement — everything in `Tests/Integration/Orkes/`, plus