CCOR-13193 - run integration(e2e) tests against latest oss conductor + v4 server by chrishagglund-ship-it · Pull Request #151 · conductor-oss/csharp-sdk

chrishagglund-ship-it · 2026-06-12T15:51:26Z

In this PR:

oss latest in a small docker stack in the runner
matrix to test against v4 and v5 for the two sets of integration tests
fix for missing test fixture
correct misshapen data structures

regarding the correction of misshapen data structures, it is technically a breaking change, and there is a question then of if this should be bugfix (eg 1.2.1) or major version (2.0.0). I lean towards the "type was impossible/useless" bugfix camp.

Breaking change: `WorkflowTask.OnStateChange` model corrected to match server contract

What breaks

Conductor.Client.Models.WorkflowTask.OnStateChange changes type:

Before: Dictionary<string, StateChangeConfig>
After: Dictionary<string, List<StateChangeEvent>>

Consequences for callers:

The OnStateChange property and the WorkflowTask(...) constructor's onStateChange
parameter change type (source-breaking at compile time).
StateChangeConfig and the StateChangeEventType enum are removed.
StateChangeEvent moves from the global namespace into Conductor.Client.Models
(callers now need using Conductor.Client.Models;).

This surfaces on any call that sends or receives a WorkflowDef containing tasks with
onStateChange, e.g. MetadataResourceApi.UpdateWorkflowDefinitions(...)
(PUT /metadata/workflow) and MetadataResourceApi.Get(...) (GET /metadata/workflow/{name}).

Why the old type was wrong

onStateChange is serialized as part of WorkflowDef over the metadata REST API. The
authoritative server model is Map<String, List<StateChangeEvent>>, where the map key is
the event name (onStart, onSuccess, …) and each StateChangeEvent is { type, payload }:

OSS: conductor/.../common/metadata/workflow/WorkflowTask.java:158 (field),
StateChangeEvent.java:25-32
Orkes: orkes-conductor/failover/conductor/.../common/metadata/workflow/WorkflowTask.java:158,
StateChangeEvent.java:25-32 (identical; Orkes reuses the OSS common model)

There is no StateChangeConfig type anywhere on the server side.

So the server wants:

"onStateChange": { "onStart": [ { "type": "...", "payload": {...} } ] }

The old C# StateChangeConfig produced:

"onStateChange": { "": { "type": ["onStart"], "events": [...] } }

That's wrong on both read and write — wrong key (empty string instead of the event name)
and wrong value shape (a wrapper object with type/events instead of a bare list of
events). It only ever "worked" because nothing round-tripped it against a server that
enforced the shape. The new Dictionary<string, List<StateChangeEvent>> is an exact match
to the server model.

So: bug fix, or breaking change?

Both. The type was never correct, so this is fundamentally a bug fix — but because it
changes a public type, it's source-breaking. Flagging for the team to decide how to ship:

Major version bump (treat strictly as a breaking change), or
Minor/patch as a bugfix (argument: the old shape never produced valid requests, so no
correct code could have depended on it), or
Gate the new shape behind a feature flag / opt-in if we want a migration window.

The PR author favors minor/patch bugfix 🙂

codecov · 2026-06-12T15:55:17Z

Codecov Report

❌ Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
Conductor/Api/EnvironmentResourceApi.cs	40.00%	6 Missing ⚠️

Flag	Coverage Δ
unittests	`3.69% <72.72%> (+0.37%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
Conductor/Client/Models/EnvironmentVariable.cs	`100.00% <100.00%> (ø)`
Conductor/Client/Models/StateChangeEvent.cs	`100.00% <100.00%> (ø)`
Conductor/Client/Models/WorkflowTask.cs	`19.00% <100.00%> (+19.00%)`	⬆️
Conductor/Api/EnvironmentResourceApi.cs	`1.34% <40.00%> (+1.34%)`	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nthmost-orkes

Good PR overall. Verified against server source and our capabilities catalog — the onStateChange fix and the OSS CI job are both the right moves. A few things worth discussing:

`onStateChange` type fix — verified correct, ship as bugfix

Confirmed against WorkflowTask.java:158 in conductor-oss/conductor:

private @Valid Map<String, List<StateChangeEvent>> onStateChange = new HashMap<>();

And StateChangeEvent.java has exactly { String type, Map<String, Object> payload } — no StateChangeConfig anywhere on the server side. The old C# type was never correct; no valid server response could have round-tripped through it. +1 for minor/patch bugfix versioning.

OSS CI job

The new integration_tests_oss job is the right approach. One thing: conductoross/conductor:latest will silently break if the server ships something the SDK doesn't handle yet. Consider pinning to a specific version (e.g. conductoross/conductor:3.32.0-rc.9 or the latest stable) so CI failures are deliberate rather than surprise. Even a comment above the image line noting the intended version would help.

Scheduler exclusion — possibly too conservative

Feature!=OSSSchedulerWIP excludes SchedulerTests from the OSS run. We've been building a capabilities catalog for the OSS server, and the scheduler is fully implemented in OSS 3.32.0-rc.9 with complete Flyway migrations for SQLite, MySQL, and PostgreSQL backends. The exclusion may be warranted if the tests use Orkes-specific API paths or auth-gated endpoints, but it's worth a quick check — the OSS scheduler surface should cover the basics that the integration tests exercise.

If the tests fail against OSS because of missing endpoints (e.g. a scheduler search API that only exists in Orkes Enterprise), documenting that in the OSSSchedulerWIP annotation or a comment in the test class would help the next person know why it's excluded.

Missed opportunity while touching `WorkflowTask`

Since this PR changes WorkflowTask.OnStateChange, it's a natural place to address WorkflowTaskTypeEnum gaps we found in our SDK audit (#161). The enum currently ends at WAITFORWEBHOOK = 30 and is missing NOOP, AGENT, GET_AGENT_CARD, and CANCEL_AGENT. Not a blocker for this PR, but worth a follow-up issue if there isn't one already.

The rest of the changes (EnvironmentVariable model, EnvironmentResourceApi deserialization fix, v4 matrix) look correct. The new tests in EnvironmentAndStateChangeModelTests cover the new type shapes cleanly.

nthmost-orkes · 2026-07-17T22:23:38Z

+          cat <<'EOF' > docker-compose-oss.yaml
+          services:
+            conductor-server:
+              image: conductoross/conductor:latest


Did you mean to use latest here? If a new OSS release drops mid-PR this could break the job for reasons unrelated to the SDK change. Should we pin to a specific version tag (e.g. conductoross/conductor:3.22.0)?

let's use a org-wide env var to set which version to use, for now i have set that up as latest . I've also added workflow_dispatch option to the action so that a custom oss version could be tested against without changing the org-wide env var value.

nthmost-orkes · 2026-07-17T22:23:38Z

+        /// Projects a list of environment variables into a name/value dictionary,
+        /// skipping any entries that are missing a name or value.
+        /// </summary>
+        internal static Dictionary<string, string> ToEnvironmentDictionary(List<EnvironmentVariable> variables)


ToEnvironmentDictionary is marked internal, but EnvironmentResourceApiUnitTests.cs calls it directly from the test assembly. Without [assembly: InternalsVisibleTo("Tests")] on the main project this will be a compile error. Either add InternalsVisibleTo or make it public static.

Thanks for flagging this — I looked into it, and I think this one is a false alarm.

The key detail is that InternalsVisibleTo keys off the assembly name, not the namespace. The test's namespace is Tests.Api, but the test assembly is conductor-csharp.test (from Tests/conductor-csharp.test.csproj, which doesn't override <AssemblyName>).

The main project already grants access to exactly that assembly:

 <ItemGroup> <InternalsVisibleTo Include="conductor-csharp.test" /> </ItemGroup>

Indeed, without this xml InternalsVisibleTo there would be a compile error

nthmost-orkes · 2026-07-17T22:23:38Z

+                    $"Workflow status is {workflow.Status} and status of last task {lastTask.ReferenceTaskName} is {lastTask.Status}");
+
+                // Mark the WAIT task as completed by calling Task completion API
+                var taskResult = new TaskResult


taskResult is declared but never passed to anything — UpdateTaskSync is called with individual arguments on the next line. Dead variable; either wire it up or remove it.

nthmost-orkes · 2026-07-17T22:23:38Z

+                System.Threading.Thread.Sleep(500);
+            }
+
+            var final_ = _taskClient.GetTask(taskId);


final is not a reserved keyword in C#, so the trailing underscore is unnecessary. Just var final.

nthmost-orkes · 2026-07-17T22:23:38Z

+    "schemaVersion": 2,
+    "restartable": true,
+    "workflowStatusListenerEnabled": false,
+    "ownerEmail": "viren@orkes.io",


Viren has been immortalized in the test fixture. Flattering, but maybe use something like "test@conductoross.io" so future readers don't wonder if he's personally on-call for the C# SDK test suite.

…inst conductor oss

…st that needs it. setup to test against v4 and v5 in gh action

…ng comparison in favor of initial enum string parse and then compare to enum value

…for pause and resume schedule.

chrishagglund-ship-it changed the base branch from main to fix/latency-issues June 12, 2026 15:53

chrishagglund-ship-it force-pushed the e2e-with-oss branch 2 times, most recently from cb0af8b to 6ec17eb Compare June 16, 2026 16:04

chrishagglund-ship-it changed the title ~~run integration(e2e) tests against latest oss conductor~~ run integration(e2e) tests against latest oss conductor + v4 server Jun 16, 2026

chrishagglund-ship-it force-pushed the fix/latency-issues branch from 4b03a0e to b88392d Compare June 22, 2026 17:38

chrishagglund-ship-it force-pushed the e2e-with-oss branch 3 times, most recently from 223cba3 to 268df8e Compare June 22, 2026 17:52

chrishagglund-ship-it changed the base branch from fix/latency-issues to main June 22, 2026 17:53

chrishagglund-ship-it marked this pull request as ready for review July 2, 2026 18:57

chrishagglund-ship-it requested review from bradyyie and mp-orkes July 2, 2026 18:57

chrishagglund-ship-it marked this pull request as draft July 2, 2026 19:10

chrishagglund-ship-it marked this pull request as ready for review July 3, 2026 15:59

chrishagglund-ship-it changed the title ~~run integration(e2e) tests against latest oss conductor + v4 server~~ CCOR-13193 - run integration(e2e) tests against latest oss conductor + v4 server Jul 3, 2026

nthmost-orkes reviewed Jul 17, 2026

View reviewed changes

chrishagglund-ship-it added 9 commits July 21, 2026 09:10

integration/e2e testing the supported functions of existing tests aga…

d96f347

…inst conductor oss

set proper target branch for action testing

a123bb1

debugging and robustness in inexplicably failing tests

1388aa6

some task cleanup for tests

ec46595

create csharp_sync_task_variable_updates wf if it doesn't exist in te…

001e20d

…st that needs it. setup to test against v4 and v5 in gh action

improve coverage of unit tests

4080eb3

remove stale branch target

76087bd

improve a task result status verification by removing the direct stri…

d2a15ea

…ng comparison in favor of initial enum string parse and then compare to enum value

address pr feedback, implement suggestions

2303ec0

chrishagglund-ship-it force-pushed the e2e-with-oss branch from 8162bf8 to 2303ec0 Compare July 21, 2026 16:18

chrishagglund-ship-it added 2 commits July 21, 2026 13:01

temporary handling of get/put mismatch between oss and orkes servers …

1b05c67

…for pause and resume schedule.

update comment to better reflect the truth of the orkes server change

24a6f43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CCOR-13193 - run integration(e2e) tests against latest oss conductor + v4 server#151

CCOR-13193 - run integration(e2e) tests against latest oss conductor + v4 server#151
chrishagglund-ship-it wants to merge 11 commits into
mainfrom
e2e-with-oss

chrishagglund-ship-it commented Jun 12, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

nthmost-orkes left a comment

Uh oh!

nthmost-orkes Jul 17, 2026

Uh oh!

chrishagglund-ship-it Jul 21, 2026

Uh oh!

nthmost-orkes Jul 17, 2026

Uh oh!

chrishagglund-ship-it Jul 21, 2026 •

edited

Loading

Uh oh!

nthmost-orkes Jul 17, 2026

Uh oh!

nthmost-orkes Jul 17, 2026

Uh oh!

nthmost-orkes Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chrishagglund-ship-it commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

In this PR:

Breaking change: WorkflowTask.OnStateChange model corrected to match server contract

What breaks

Why the old type was wrong

So: bug fix, or breaking change?

Uh oh!

codecov Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nthmost-orkes left a comment

Choose a reason for hiding this comment

onStateChange type fix — verified correct, ship as bugfix

OSS CI job

Scheduler exclusion — possibly too conservative

Missed opportunity while touching WorkflowTask

Uh oh!

nthmost-orkes Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chrishagglund-ship-it Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

nthmost-orkes Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

chrishagglund-ship-it Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nthmost-orkes Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

nthmost-orkes Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

nthmost-orkes Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chrishagglund-ship-it commented Jun 12, 2026 •

edited

Loading

Breaking change: `WorkflowTask.OnStateChange` model corrected to match server contract

codecov Bot commented Jun 12, 2026 •

edited

Loading

`onStateChange` type fix — verified correct, ship as bugfix

Missed opportunity while touching `WorkflowTask`

chrishagglund-ship-it Jul 21, 2026 •

edited

Loading