ci: don't cancel same-repo runs when 'safe to test' label is added - #31613
ci: don't cancel same-repo runs when 'safe to test' label is added#31613chirag-madlani wants to merge 1 commit into
Conversation
Same-repo PRs already run with full trust under pull_request / pull_request_target, so labeling them "safe to test" adds no new capability. The previous concurrency expression cancelled the in-flight synchronize run and kicked off a fresh identical run — wasted CI. Guard the "safe to test" cancel with a fork check so only fork PRs (whose head repo differs from the base) restart on the label. All other events (synchronize, opened, reopened, ready_for_review, merge_group, dispatch) continue to cancel stale runs as before. Non-"safe to test" labels remain non-cancelling. Applied uniformly to 18 workflows that share the pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
Code Review ✅ ApprovedGuards workflow concurrency cancellation predicates with a fork check to prevent same-repo runs from being cancelled when the 'safe to test' label is added. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
✅ Playwright Results — workflow succeededValidated commit ✅ 551 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 46m 45s ⏱️ Max setup 3m 48s · max shard execution 18m 43s · max shard-job elapsed before upload 22m 10s · reporting 4s 🌐 213.98 requests/attempt · 2.83 app boots/UI scenario · 8.64% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
🚦 Removed from the merge queue —
|
Describe your changes:
Fixes: N/A — CI hardening for the label-guarded workflows.
Same-repo PRs already run with full trust under
pull_request/pull_request_target, so labeling onesafe to testgrants no new capability. The previousconcurrency.cancel-in-progressexpression treated anylabeled: safe to testevent as a cancel-trigger, which killed the in-flightsynchronizerun and started an identical fresh run — pure CI waste, and visually a spurious "re-run" on the PR checks. Non-safe to testlabels (e.g.To release) also produced confusing skipped-run entries because they entered the same concurrency group and either queued behind or cancelled a pending real run.This PR guards the
safe to testbranch of the concurrency predicate with a fork check (github.event.pull_request.head.repo.full_name != github.repository). Now only fork PRs restart on the label — same-repo PRs preserve the running work. All other events (synchronize,opened,reopened,ready_for_review,merge_group,workflow_dispatch) still cancel stale runs as before, and non-safe to testlabels remain non-cancelling.Applied uniformly to 18 workflows that share the pattern.
Type of change:
High-level design:
N/A — small change. One-line edit per workflow inside the existing
concurrency.cancel-in-progressexpression:becomes
Files touched (18):
airflow-apis-tests,data-access-request-e2e,integration-tests-mysql-elasticsearch,integration-tests-postgres-elasticsearch-redis,integration-tests-postgres-opensearch,java-checkstyle,maven-build-collate,maven-sonar-build,openmetadata-service-unit-tests,playwright-knowledge-graph-postgresql-e2e,playwright-postgresql-e2e,py-checkstyle,py-operator-build-test,py-tests-postgres,py-tests,typescript-type-generation,ui-checkstyle,yarn-coverage.playwright-knowledge-graph-postgresql-e2e.ymlpreviously had a barecancel-in-progress: true; it now uses the same guarded expression asplaywright-postgresql-e2e.yml(both handlepull_request+pull_request_target).Rejected alternatives: splitting the concurrency group so non-
safe to testlabel runs never touch the real pipeline (fix A) — deferred; this PR fixes the highest-impact case (wasteful cancel-and-restart on same-repo labeling).Tests:
Use cases covered
labeled: safe to test→ in-flightsynchronizerun is not cancelled.labeled: safe to test→ gated run is cancelled and privileged run starts (unchanged).safe to teston any PR → in-flight run is not cancelled (unchanged).synchronize/opened/reopened/ready_for_review→ stale runs still cancelled (unchanged).merge_group/workflow_dispatch→ still cancel stale runs (unchanged).Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
python3 -c "import yaml; yaml.safe_load(open(...))"on each edited file.grep— all 18 files now use the guarded predicate.UI screen recording / screenshots:
Not applicable.
Checklist:
Greptile Summary
This PR updates concurrency cancellation predicates across 18 CI workflows so adding
safe to testonly cancels an existing run for fork PRs.Confidence Score: 5/5
The PR appears safe to merge, with no actionable correctness or security failures identified in the changed workflow predicates.
The new repository comparison is reached only for labeled PR events, fork label events continue cancelling stale runs, and all other supported events retain their previous cancellation behavior.
Important Files Changed
pull_request_targetcancellation predicate without changing job authorization.pull_requestevent path.Reviews (1): Last reviewed commit: "ci: don't cancel same-repo runs when saf..." | Re-trigger Greptile