Unblock integration CI: stop leaking task defs, drop exact-404-wording assert - #475
Closed
ling-senpeng13 wants to merge 3 commits into
Closed
Unblock integration CI: stop leaking task defs, drop exact-404-wording assert#475ling-senpeng13 wants to merge 3 commits into
ling-senpeng13 wants to merge 3 commits into
Conversation
ling-senpeng13
marked this pull request as draft
August 12, 2026 21:30
ling-senpeng13
changed the title
test_all asserted the deleted-workflow 404 read exactly "workflow with id: <id> not found." The server now returns "No execution found for id: <id>", so the test fails on every branch (reproduced on fix/pin-mcp-below-2 and fix/13-hierarchical-agents-demo; main last went green Aug 7, before the change). Assert the durable part instead: a 404 whose message names the execution.
The sdkdev account has hit its cap: 402 System has reached the maximum allowed Task Definitions limit of 1000. so registration now fails for every branch, and test_05_verify_task_definitions 404s on the def that could not be registered. Three suites register RUN_ID-suffixed task defs via worker_task(register_task_def=True) and never remove them, leaking ~11 per run across all four integration jobs: test_comprehensive_e2e (5 tasks + 1 workflow), test_async_lease_extension (4 + 4), test_lease_extension (2 + 2, which had no tearDownClass at all). Cleanup is best-effort — a failed unregister warns rather than reddening a passing suite. scripts/prune_leaked_test_task_defs.py clears the backlog already on the server; this stops the leak but cannot free the 1000 defs already there. It is a dry run unless given --delete, and only matches the suites' own prefixes followed by a run id, so no hand-registered def is a candidate.
Unregistering on teardown stops the leak but cannot free the ~1000 defs
already registered, so the account stays at its cap and every branch keeps
getting 402 on registration — no run gets far enough to clean up after
itself. Prune the stale leftovers before any test registers.
Only names matching an integration suite's prefix followed by a run id are
candidates, and only ones older than 2h: four buckets run in parallel against
one server, so a def a concurrent run may still be using is off limits (as is
one with no createTime, whose age is unknown). The reclaim prints how many it
freed, so a run that is still capped says so instead of failing obscurely.
Verified against a local server: reclaim deletes a registered
sync_basic_<run id> and reports the count; test_comprehensive_e2e (8 passed),
test_lease_extension and test_async_lease_extension pass with cleanup leaving
nothing behind; the deleted-workflow 404 assert holds on this server's third
wording of that message ("No such workflow found by id: <id>").
scripts/prune_leaked_test_task_defs.py now shares the matcher rather than
duplicating it, and grew --include-recent for pruning by hand when no run is
in flight.
ling-senpeng13
force-pushed
the
fix/relax-deleted-workflow-404-assert
branch
from
August 12, 2026 22:05
73255af to
071131e
Compare
manan164
added a commit
that referenced
this pull request
Aug 13, 2026
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
added a commit
that referenced
this pull request
Aug 13, 2026
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.
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
1 participant
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.
Two things break integration CI on every branch, neither from the code under test.
1. Server changed its 404 wording.
test_allrequired exactlyworkflow with id: <id> not found.; the server now saysNo execution found for id: <id>. Assert the durable part — a 404 naming the execution.2. The sdkdev account hit its task-def cap.
Three suites register RUN_ID-suffixed task defs via
worker_task(register_task_def=True)and never remove them — ~11 leaked per run, across four jobs, on every PR. Added best-efforttearDownClasscleanup (test_lease_extensionhad notearDownClassat all).This stops the leak but does not free the 1000 defs already registered, so CI stays red until someone with sdkdev credentials runs
scripts/prune_leaked_test_task_defs.py --delete(dry run by default; only matches the suites' own prefixes + a run id).