Unblock CI: stop asserting server prose, fix task-def leak and retry gaps - #490
Merged
Conversation
test_all asserted the exact 404 body for a deleted workflow:
assert str(e.message).lower() == "workflow with id: {} not found."
The sdkdev v5 deployment was rolled between Aug 10 and Aug 12 and now
answers "No execution found for id: <uuid>". No SDK or test code changed
across that boundary -- integration-test (test-all) was green on Aug 10
08:01 and red from Aug 12 19:35, and rest.py surfaces the server's
`message` field verbatim -- so the assert broke on a server-side prose
change alone. It has kept CI red on main and on every open PR since.
Assert the status code, which is the actual contract, and only that the
id appears in the message.
Also closes a vacuous pass in the same block: the try had no else or
fail, so a get_workflow that did not raise satisfied the test silently.
Raise instead if the deleted workflow is still readable -- AssertionError
is not an ApiException, so it propagates past the handler.
Two failure classes have been turning CI red without any SDK change. 1. Exact-prose 404 asserts. Seven sites compared e.message against the server's exact English sentence. The 5.3.3 -> 5.5.0 upgrade of the shared sdkdev server reworded one of them and broke main. Replaced with _assert_not_found, which asserts the 404 status -- the actual contract -- and only that the identifier appears in the message. It also fails when the resource is still readable after a delete. The bare try/except at each site passed silently in that case, so a delete that did not take effect would not have been caught. 2. Read-after-write on queue size. get_queue_size_for_task was asserted immediately after starting or draining work, but the server enqueues and indexes asynchronously, so the count intermittently came back stale (assert 0 == 2). Wrapped in _await_value, which polls up to 15s for the expected value and returns the last one seen so a real mismatch still reports the actual number. Not touched: __test_workflow_rate_limit has the same read-after-write shape but is dead code -- nothing in run() calls it.
manan164
changed the title
Folds in the remaining deterministic CI failure causes so one PR covers all of them. Task-def quota (402). The integration suites register per-run task defs and never removed them, so on the shared server they accumulated to the 1000 cap and registration then answered 402 for every branch -- which is also what produced the test_05_verify_task_definitions 404s. Adds tearDownClass cleanup to the suites that leak, a conftest session hook that reclaims what earlier runs already left behind, and a manual prune script. Reclaim only touches names matching a known test prefix plus a run id, and only those older than two hours, so a concurrent run never has a def deleted from under it. Salvaged from the closed PR #475. 423 write contention. "Workflow is currently being updated, please retry" is the server asking for a retry, but it is scoped to retry_scenario only, not retry_on_transient: replaying a single non-idempotent call (start_workflow, signal) risks double-executing a write that landed and only contended on the read-back, whereas a scenario re-runs from the top and rebuilds its own state. Service registry 502. That suite called its scenarios bare, so one 502 from the proxy failed the whole run even though is_transient already covers gateway 5xx. Wrapped in retry_scenario with the same shared deadline the other aggregate suites use. Not fixed: test_v2_fallback_intg "workflows did not complete in time". It already retries transients, so this is completion timing, not a transient API error -- possibly real 5.5.0 slowness. Raising its deadline blind would hide that.
manan164
changed the title
manan164
force-pushed
the
fix/deleted-workflow-404-assert
branch
from
August 13, 2026 15:49
8aa5903 to
5f3ba52
Compare
manan164
changed the title
Both intermittent failures assert only on workflow status, so a red run says "still RUNNING" or "5 pending" and nothing about why. Dump each task's status, domain, pollCount and workerId on the failure path. That separates the possibilities in one run instead of by hypothesis: SCHEDULED means queued but never polled (wrong queue or domain, or no live worker for the type), IN_PROGRESS means polled and leased but never updated, and no tasks at all means the workflow was never decided. Diagnostics only -- no behaviour change.
Nothing runs it. The conftest session hook already reclaims stale leaked task defs, which is what actually keeps the account under its cap; the script was a hand-run convenience that came along with the #475 salvage and is not needed for CI to pass.
The seven signal scenarios started a workflow, slept 0.5-1.0s, then signalled and asserted on the returned strategy. Every signal strategy describes the task the workflow is parked on, so if the workflow has not got there yet the server answers with no responseType and the assert reads "Expected BLOCKING_TASK, got None". On a loaded shared server one second is not enough, which is why it failed intermittently. Poll for the precondition -- a task in SCHEDULED or IN_PROGRESS -- up to 30s, and give up early if the workflow is already terminal so the caller's own assertion reports the real state. Logs the tasks it saw when the wait times out.
Task-level diagnostics from a red run showed these are two different problems, not the one I assumed: scenario_decorated_workers: task SCHEDULED, pollCount=0, workerId=None after sleep(15), while both decorated workers were demonstrably active and polling every 100ms. The server simply had not handed the task out inside that window. This is the same false negative the batch-completion budget in this file was already raised to fix, so use that budget here and poll to terminal. test_v2_fallback_intg: 4 of a workflow's 5 tasks COMPLETED with the last IN_PROGRESS on a live worker (pollCount=1, workerId set) when the 60s budget expired -- it was progressing, not stuck. Raised to 120s. A task still IN_PROGRESS after that is a real lost update rather than slowness, and the diagnostic prints it.
retry_scenario re-runs a scenario from the top on a transient blip, but the tag sub-tests assumed a clean slate. A run showed exactly that: transient (0) in test_task_lifecycle running scenario test_task_lifecycle (attempt 2) Attempt 1 had already added tags, so attempt 2's "add one tag, now there is exactly one" assertion counted the leftovers and failed with `assert 2 == 1`. Clear the tags first so the assertions hold on any attempt.
Contributor
|
I think the current CI introduces an improper dependency on To make CI consistent, we should run the OSS-compatible tests against a pinned local server (as agent-e2e.yml already does), and keep only the Orkes-specific ones on sdkdev as non-blocking. |
mp-orkes
approved these changes
Aug 14, 2026
Contributor
|
Merging this PR to fix CI. It's read on every PR now.
We want to test using both OSS and Orkes Conductor. If test don't pass it SHOULD BE blocking. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sdkdev was rolled 5.3.3 → 5.5.0, pulling in orkes-io/orkes-conductor#3781, which moved the workflow-not-found 404 to a different handler and reworded it, breaking a test that string-compared the server's prose — no SDK commit was involved.
The rest were pre-existing test defects rather than server regressions: exact-prose 404 asserts at 7 sites, read-after-write on queue size and tags, fixed sleeps before signalling (
Expected BLOCKING_TASK, got None), tag scenarios that were not re-runnable underretry_scenario, the 402 task-def leak, 423 write contention, and a 502 in the service-registry suite.Verified with 10 consecutive green CI runs on
d84f593a, the runner set to halt on the first failure.Not fixed: a workflow whose six tasks all reported COMPLETED was still not finalized by the server, which points at the 5.5.0 sweeper changes (#3754, #3775) rather than the SDK; the task-state diagnostics added here will surface it if it recurs.