ci(sdk-tests): cancel superseded runs on the same PR - #1937
Conversation
|
There was a problem hiding this comment.
TASTE.md review: checked T-1 through T-74 — 0 violations. The only change is a concurrency block in .github/workflows/sdk_tests.yml; it touches no packages/js-sdk or packages/python-sdk public API surface, so none of the SDK design principles apply.
Package ArtifactsBuilt from dc9e451. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.52.1-charlie-sdk-tests-cancel-superseded.0.tgzCLI ( npm install ./e2b-cli-2.21.1-charlie-sdk-tests-cancel-superseded.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.1-charlie-sdk-tests-cancel-superseded.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-charlie-sdk-tests-cancel-superseded.0.tgzPython SDK ( pip install ./e2b-2.52.0+charlie.sdk.tests.cancel.superseded-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.1+charlie.sdk.tests.cancel.superseded-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.6.0+charlie.sdk.tests.cancel.superseded-py3-none-any.whl |
Rapid pushes to a PR left several SDK Tests runs overlapping, each building the same shared test templates in parallel. Only the latest run matters, so the earlier ones were wasted CI time and duplicate build load. Group runs per PR and cancel the in-flight run when a newer push arrives. workflow_dispatch runs are grouped by ref but never cancelled.
4218dd1 to
149c1a2
Compare
There was a problem hiding this comment.
Looks good, straightforward CI config change.
What was reviewed: the new concurrency block in .github/workflows/sdk_tests.yml — group key (workflow name + PR number, falling back to ref) and the cancel-in-progress expression scoped to pull_request events only. Confirmed workflow_dispatch runs are excluded from cancellation as intended (the only candidate issue raised — two manual dispatches on the same ref not cancelling each other — is an accepted, explicitly documented tradeoff, not a bug).
Extended reasoning...
The change adds a single concurrency block (group + cancel-in-progress) to a GitHub Actions workflow file, with no touch to application code, auth, or data handling. The expression logic is standard and correct for the stated goal (dedupe overlapping PR pushes while leaving workflow_dispatch runs uncancelled), the change is small and self-contained, and no outstanding reviewer objections exist in the timeline.
This review covers commit 4218dd1, which is no longer the latest commit on this pull request; later commits are not covered by it.
A rerun keeps its PR number, so an older commit's rerun joined the PR's group and cancelled the current head's run. Manual dispatches on one ref shared a group too, and GitHub keeps only one pending run per group, so a third dispatch silently replaced the second. Only first attempts of pull_request runs share the per-PR group now; reruns and dispatches each get a group keyed by run ID.
Summary
A new push to a PR now cancels that PR's in-flight
SDK Testsrun. Only first attempts ofpull_requestruns share the per-PR group; reruns andworkflow_dispatchruns each get their own group, so a rerun of an older commit can't cancel the current head's run and manual dispatches never replace each other.Why
Rapid pushes to a PR leave several runs overlapping. Each one builds the same shared test templates (e.g.
e2b-tags-test) in parallel, across the 7 JS runtime legs and the Python sync and async suites. Only the latest run's result matters, so the earlier runs are wasted CI time and duplicate build load.Possible follow-ups, for discussion:
e2b-tags-testmain/dispatch (this changes what the requiredSDK Tests Statuscheck covers)Verification
actionlint .github/workflows/sdk_tests.ymlpasses.cancelledin itsSDK Tests Status, but the required check is evaluated on the PR's latest head SHA, whose run is unaffected.🤖 Generated with Claude Code