diff --git a/.agents/skills/authoring-ci-workflows/SKILL.md b/.agents/skills/authoring-ci-workflows/SKILL.md index 3c21d647a49e..669de37d54a0 100644 --- a/.agents/skills/authoring-ci-workflows/SKILL.md +++ b/.agents/skills/authoring-ci-workflows/SKILL.md @@ -16,7 +16,7 @@ The linters own the mechanical rules (below); this skill is the **judgment calls - Copy from a canonical file rather than from memory. `ci-paths-filter.yml` is the smallest complete example (triggers, concurrency, timeout, app token, Depot runner); - `ci-backend.yml` is the reference for the heavy patterns (bounded-depth checkout, per-SHA concurrency, draft/ready, sharding). + `ci-backend.yml` is the reference for the heavy patterns (bounded-depth checkout, per-SHA concurrency, selective PR runs vs the merge-queue full gate, sharding). - Related skills — reach for these instead of duplicating them here: - `/gating-production-deploys` — any job that pushes a prod image or dispatches a Charts deploy. - `/managing-github-actions-secrets` — creating the GitHub App / secret a workflow reads. @@ -211,11 +211,13 @@ PR-scoped cache writes nobody else can read just fragment the 10 GB LRU cap. New Depot labels must be added to the allow-list in `.github/actionlint.yaml` or actionlint fails. Details: `/depot-github-runners`. -## Draft vs ready-for-review +## Selective PR runs vs the merge-queue full gate -Most commits land before a PR is marked ready, and drafts can't merge — so heavy suites should run a narrowed subset on drafts and the full matrix on `ready_for_review` (the merge gate). -Add `ready_for_review` to the `pull_request` types, and make aggregator "... Tests Pass" jobs treat `skipped` as success so drafts still report. -Foot-gun: if a `select-tests` job is cancelled mid-flight, its `mode` output is empty — normalize empty-mode **on a draft** to `skip`, or the draft grabs the full matrix and serializes the ready run behind it. +The Trunk merge queue is the merge gate: it tests each queued PR on a `trunk-merge/**` draft PR that runs the full suites, and ejects the PR when they fail. +Regular PRs (draft or ready) therefore run a narrowed subset for fast feedback — heavy suites should gate their selective path on `!startsWith(github.head_ref, 'trunk-merge/')` and run the full matrix on `trunk-merge/**` PRs, master pushes, and dispatch. +Make aggregator "... Tests Pass" jobs treat `skipped` as success so selective runs still report, and keep a force label (`run-ci-backend` / `run-ci-frontend`) so a full run can be demanded on any PR. +Foot-gun: if a `select-tests` job is cancelled mid-flight, its `mode` output is empty — normalize empty-mode **on a regular PR** to `skip`, or the PR grabs the full matrix and holds the per-branch concurrency slot for ~30 min. +Note the queue's test PRs are drafts — never use `pull_request.draft` to decide selective vs full; a draft check would give the merge gate the narrowed run. ## Backwards-compat with unrebased PRs diff --git a/.depot/workflows/ci-backend.yml b/.depot/workflows/ci-backend.yml index 059700453ef7..1182a2880ad8 100644 --- a/.depot/workflows/ci-backend.yml +++ b/.depot/workflows/ci-backend.yml @@ -96,14 +96,14 @@ on: description: ClickHouse server version. Leave blank for default type: string pull_request: - # Draft PRs run the snob-selected Django subset (≤3 shards); turbo-tests - # still skip drafts. Ready PRs run the full matrices — the merge gate — - # and ready_for_review re-triggers them when a PR leaves draft. Add the - # `run-ci-backend` label to force the full matrices on a draft; the - # `no-ci` label silences the workflow on a draft entirely. No - # labeled/unlabeled triggers, so a label takes effect from the next push - # or from ready_for_review. Mirrors canonical. - types: [opened, synchronize, reopened, ready_for_review] + # PRs (draft or ready) run the snob-selected Django subset (≤3 shards); + # turbo-tests skip PRs. The full matrices run on the merge queue's + # trunk-merge/** draft PRs — the merge gate. Add the `run-ci-backend` + # label to force the full matrices on a PR; the `no-ci` label silences + # the workflow on a draft entirely. No labeled/unlabeled or + # ready_for_review triggers, so a label takes effect from the next push. + # Mirrors canonical. + types: [opened, synchronize, reopened] concurrency: # PRs: one active run per branch, cancel stale. Push: per-SHA so master # pushes never cancel each other (check-migrations always completes). @@ -549,13 +549,13 @@ jobs: echo "run_legacy=$(echo "$RESULT" | jq -r '.run_legacy')" >> $GITHUB_OUTPUT echo "django_shards=$(echo "$RESULT" | jq -c '.django_shards // empty')" >> $GITHUB_OUTPUT - # Pick which Django tests to run on draft PRs. Drafts get the snob-selected - # subset for fast feedback; the full matrix runs once the PR is marked ready - # for review, and that ready run is the merge gate. When selection can't be - # trusted on a draft (legacy graph impact, turbo-discover or selector failure, - # a selector full-run signal), the draft skips the heavy matrices entirely — - # the pre-selection draft behavior — and defers to the ready full run. - # hogli-lint: not-a-required-gate - selects draft coverage and emits no required check. + # Pick which Django tests to run on PRs. PRs get the snob-selected subset for + # fast feedback; the full matrix runs on the merge queue's trunk-merge/** test + # PRs, and that queue run is the merge gate. When selection can't be trusted + # on a PR (legacy graph impact, turbo-discover or selector failure, a selector + # full-run signal), the PR skips the heavy matrices entirely and defers to the + # merge-queue full run. + # hogli-lint: not-a-required-gate - selects PR coverage and emits no required check. # # Depot shadow note: canonical uploads /tmp/selection.json for its # test-selection-verdict job. The depot shadow strips that job (and all @@ -563,15 +563,14 @@ jobs: select-tests: name: Select tests needs: [changes, turbo-discover] - # Only draft PRs do selective runs; ready PRs and pushes always run full, - # which build_django_matrix falls back to + # Only regular PRs do selective runs; merge-queue trunk-merge/** PRs and + # pushes always run full, which build_django_matrix falls back to # when select-tests is skipped (empty MODE). The run-ci-backend label - # forces the full matrices on a draft. + # forces the full matrices on a PR. # Trunk's merge-queue branches open as draft PRs, but their run IS the # merge gate, so they must never get the narrowed selection. if: | github.event_name == 'pull_request' && - github.event.pull_request.draft == true && !startsWith(github.head_ref, 'trunk-merge/') && !contains(github.event.pull_request.labels.*.name, 'run-ci-backend') && needs.changes.outputs.backend == 'true' @@ -618,7 +617,7 @@ jobs: if [[ "$RUN_LEGACY" == "true" ]]; then # turbo-discover detected product->legacy graph impact; the # diff-based selector can't see this, so its subset would be - # incomplete. Skip the draft matrices; the ready run is full. + # incomplete. Skip the selective matrices; the merge-queue run is full. skip=true elif [[ "$TURBO_RESULT" != "success" && "$TURBO_RESULT" != "skipped" ]]; then # Conservative — turbo-discover failed. @@ -634,7 +633,7 @@ jobs: id: select # continue-on-error so a selector or git fetch failure doesn't fail this # job — a failed select-tests would leave MODE empty and build_django_matrix - # would fall back to an expensive full matrix on a draft. Classify reads + # would fall back to an expensive full matrix on a PR. Classify reads # steps.select.outcome (pre-continue-on-error) and emits mode=skip instead. continue-on-error: true if: steps.fallback.outputs.skip == 'false' @@ -678,8 +677,8 @@ jobs: } >> "$GITHUB_OUTPUT" } - # Untrusted selection on a draft skips the heavy matrices (the - # pre-selection draft behavior); the ready-for-review run is full. + # Untrusted selection skips the heavy matrices; the merge-queue + # run is full. fall_back_to_skip() { echo "mode=skip" >> "$GITHUB_OUTPUT" echo "core_files=" >> "$GITHUB_OUTPUT" @@ -696,14 +695,14 @@ jobs: fi if [[ "$SELECT_OUTCOME" != "success" ]] || [[ ! -s /tmp/selection.json ]]; then - echo "::warning::shadow selector did not produce output; draft skips heavy matrices (full run happens on ready for review)" + echo "::warning::shadow selector did not produce output; skipping heavy matrices (full run happens in the merge queue)" fall_back_to_skip selector_error exit 0 fi full_run_reasons=$(jq -r '.ast.full_run_reasons | length' /tmp/selection.json) if [[ "$full_run_reasons" -gt 0 ]]; then - echo "Selector requested a full run; draft defers it to ready for review:" + echo "Selector requested a full run; deferring it to the merge queue:" jq -r '.ast.full_run_reasons[]' /tmp/selection.json fall_back_to_skip full_run_requested exit 0 @@ -778,12 +777,14 @@ jobs: # Runs product tests in parallel — one matrix job per group # Each job gets its own runner + Docker stack, so no shared DB conflicts # Small products (< 50 tests) are grouped into a single job to avoid setup overhead + # Skipped on regular PRs — the merge queue's trunk-merge/** run is the full gate; + # the run-ci-backend label forces them on a PR. # A status function is required to inspect dependency results after failures. # Use !cancelled(), not always(), so superseded matrix jobs still receive cancellation. turbo-tests: needs: [changes, turbo-discover, detect-snapshot-mode, build-product-test-matrix, get_clickhouse_versions] if: >- - !cancelled() && (github.event.pull_request.draft != true || startsWith(github.head_ref, 'trunk-merge/') || contains(github.event.pull_request.labels.*.name, 'run-ci-backend')) && needs.turbo-discover.result == 'success' && needs.build-product-test-matrix.result == 'success' && needs.build-product-test-matrix.outputs.include != '[]' && needs.build-product-test-matrix.outputs.include != '' + !cancelled() && (github.event_name != 'pull_request' || startsWith(github.head_ref, 'trunk-merge/') || contains(github.event.pull_request.labels.*.name, 'run-ci-backend')) && needs.turbo-discover.result == 'success' && needs.build-product-test-matrix.result == 'success' && needs.build-product-test-matrix.outputs.include != '[]' && needs.build-product-test-matrix.outputs.include != '' runs-on: depot-ubuntu-24.04 timeout-minutes: 40 name: Product tests (${{ matrix.group }}, events schema ${{ matrix.new-events-schema && 'json' || 'legacy' }}) @@ -1286,31 +1287,31 @@ jobs: RUN_POE: ${{ needs.select-tests.outputs.run_poe }} RUN_TEMPORAL: ${{ needs.select-tests.outputs.run_temporal }} CORE_FILES: ${{ needs.select-tests.outputs.core_files }} - # A trunk-merge/** PR opens as a draft but is the merge gate, and - # select-tests deliberately does not run for it. Reporting it as a - # draft here would turn that empty MODE into "skip" and build an - # empty matrix, so the queue would merge having run no Django tests - # at all. Not-draft makes the empty MODE fall through to the full - # matrix, which is what the queue has to run. - IS_DRAFT: ${{ github.event.pull_request.draft && !startsWith(github.head_ref, 'trunk-merge/') }} + # trunk-merge/** is excluded because select-tests deliberately does + # not run for it. Counting it as selective would turn that empty MODE + # into "skip" and build an empty matrix, so the queue would merge + # having run no Django tests at all. Leaving it non-selective makes + # the empty MODE fall through to the full matrix, which is what the + # queue has to run. + IS_SELECTIVE_PR: ${{ github.event_name == 'pull_request' && !startsWith(github.head_ref, 'trunk-merge/') }} FORCE_FULL: ${{ contains(github.event.pull_request.labels.*.name, 'run-ci-backend') }} run: | # :NOTE: Keep shard counts/group ranges in sync with historical Django matrix tuning. # Consult #team-devex before changing. - # A draft PR must never fall back to the full matrix. select-tests only - # runs on drafts and sets MODE to "skip"/"selected"; an empty MODE on a - # draft means select-tests was cancelled or failed (typically - # ready_for_review superseding it mid-flight). Skip here and defer to the - # ready run. The run-ci-backend label intentionally forces full on a draft. - if [[ -z "$MODE" && "$IS_DRAFT" == "true" && "$FORCE_FULL" != "true" ]]; then - echo "::notice::Draft PR with no trusted Django selection — skipping heavy matrices; the ready-for-review run is the full gate" + # A regular PR must never fall back to the full matrix. select-tests only + # runs on regular PRs and sets MODE to "skip"/"selected"; an empty MODE on + # one means select-tests was cancelled or failed (typically a newer push + # superseding it mid-flight). Skip here and defer to the merge-queue run — + # the actual merge gate. The run-ci-backend label intentionally forces full. + if [[ -z "$MODE" && "$IS_SELECTIVE_PR" == "true" && "$FORCE_FULL" != "true" ]]; then + echo "::notice::PR with no trusted Django selection — skipping heavy matrices; the merge-queue run is the full gate" MODE="skip" fi if [[ "$MODE" == "skip" ]]; then - # Draft PR where selection couldn't be trusted: skip the heavy - # matrices and defer to the full run on ready for review. + # PR where selection couldn't be trusted: skip the heavy + # matrices and defer to the full run in the merge queue. echo "include=[]" >> "$GITHUB_OUTPUT" echo "Django matrix size: 0 (mode=skip)" exit 0 @@ -1538,7 +1539,7 @@ jobs: # 1. Legacy code directly changed (ee/, posthog/) # 2. OR product changes affect legacy code (turbo-discover outputs run_legacy=true) # 3. OR turbo-discover itself failed (conservative: run Django on detection failure) - # The non-empty include guard covers drafts in skip mode and selected runs + # The non-empty include guard covers PRs in skip mode and selected runs # that chose zero files — an empty matrix would otherwise fail the job. # A status function is required to inspect dependency results after failures. # Use !cancelled(), not always(), so superseded matrix jobs still receive cancellation. diff --git a/.github/workflows/ci-backend.yml b/.github/workflows/ci-backend.yml index 8f58b6664b69..3bdcad47bb87 100644 --- a/.github/workflows/ci-backend.yml +++ b/.github/workflows/ci-backend.yml @@ -13,21 +13,21 @@ on: description: ClickHouse server version. Leave blank for default type: string pull_request: - # Draft PRs run the snob-selected Django subset (≤3 shards) for fast feedback; - # turbo-tests (product tests) still skip drafts. Ready PRs run the full - # matrices — that full run is the merge gate, and ready_for_review re-triggers - # it when a PR leaves draft. To force the full matrices on a draft, add the - # `run-ci-backend` label. Cheap checks still run on drafts. + # PRs (draft or ready) run the snob-selected Django subset (≤3 shards) for + # fast feedback; turbo-tests (product tests) skip PRs. The full matrices run + # on the merge queue's trunk-merge/** draft PRs — that queue run is the merge + # gate, and the queue ejects the PR when it fails. To force the full matrices + # on a PR, add the `run-ci-backend` label. Cheap checks still run on every PR. # The `no-ci` label does the opposite: it silences this workflow on a draft # entirely (prototypes/spikes). # No labeled/unlabeled triggers: GitHub cannot filter a label trigger by name, # so every unrelated label re-ran these matrices against a commit CI had - # already covered. A label here takes effect from the next push, or from - # ready_for_review. + # already covered. No ready_for_review either, now that draft and ready run + # the same thing. A label takes effect from the next push. # The `test-new-events-schema` label opts a PR into the doubled matrices that # rerun everything with CLICKHOUSE_HOGQL_USE_NEW_EVENTS_SCHEMA=true; unlabeled # PRs run legacy-schema only (master runs always cover both). - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, synchronize, reopened] concurrency: # PRs: one active run per branch, cancel stale. Push: per-SHA so master @@ -85,7 +85,8 @@ jobs: timeout-minutes: 5 # A draft PR labeled `no-ci` skips this workflow entirely (prototypes): # every other job needs this one, and the gates treat skipped as success. - # ready_for_review re-runs everything, so the merge gate is unaffected. + # The merge queue's trunk-merge/** draft PRs never carry the label, so the + # merge gate is unaffected. if: >- github.event_name != 'pull_request' || github.event.pull_request.draft != true @@ -506,25 +507,24 @@ jobs: echo "run_legacy=$(echo "$RESULT" | jq -r '.run_legacy')" >> $GITHUB_OUTPUT echo "django_shards=$(echo "$RESULT" | jq -c '.django_shards // empty')" >> $GITHUB_OUTPUT - # Pick which Django tests to run on draft PRs. Drafts get the snob-selected - # subset for fast feedback; the full matrix runs once the PR is marked ready - # for review, and that ready run is the merge gate. When selection can't be - # trusted on a draft (legacy graph impact, turbo-discover or selector failure, - # a selector full-run signal), the draft skips the heavy matrices entirely — - # the pre-selection draft behavior — and defers to the ready full run. - # hogli-lint: not-a-required-gate - selects draft coverage and emits no required check. + # Pick which Django tests to run on PRs. PRs get the snob-selected subset for + # fast feedback; the full matrix runs on the merge queue's trunk-merge/** test + # PRs, and that queue run is the merge gate. When selection can't be trusted + # on a PR (legacy graph impact, turbo-discover or selector failure, a selector + # full-run signal), the PR skips the heavy matrices entirely and defers to the + # merge-queue full run. + # hogli-lint: not-a-required-gate - selects PR coverage and emits no required check. select-tests: name: Select tests needs: [changes, turbo-discover] - # Only draft PRs do selective runs; ready PRs and pushes always run full, - # which build_django_matrix falls back to + # Only regular PRs do selective runs; merge-queue trunk-merge/** PRs and + # pushes always run full, which build_django_matrix falls back to # when select-tests is skipped (empty MODE). The run-ci-backend label - # forces the full matrices on a draft. + # forces the full matrices on a PR. # Trunk's merge-queue branches open as draft PRs, but their run IS the # merge gate, so they must never get the narrowed selection. if: | github.event_name == 'pull_request' && - github.event.pull_request.draft == true && !startsWith(github.head_ref, 'trunk-merge/') && !contains(github.event.pull_request.labels.*.name, 'run-ci-backend') && needs.changes.outputs.backend == 'true' @@ -570,7 +570,7 @@ jobs: if [[ "$RUN_LEGACY" == "true" ]]; then # turbo-discover detected product->legacy graph impact; the # diff-based selector can't see this, so its subset would be - # incomplete. Skip the draft matrices; the ready run is full. + # incomplete. Skip the selective matrices; the merge-queue run is full. skip=true elif [[ "$TURBO_RESULT" != "success" && "$TURBO_RESULT" != "skipped" ]]; then # Conservative — turbo-discover failed. @@ -586,7 +586,7 @@ jobs: id: select # continue-on-error so a selector or git fetch failure doesn't fail this # job — a failed select-tests would leave MODE empty and build_django_matrix - # would fall back to an expensive full matrix on a draft. Classify reads + # would fall back to an expensive full matrix on a PR. Classify reads # steps.select.outcome (pre-continue-on-error) and emits mode=skip instead. continue-on-error: true if: steps.fallback.outputs.skip == 'false' @@ -630,8 +630,8 @@ jobs: } >> "$GITHUB_OUTPUT" } - # Untrusted selection on a draft skips the heavy matrices (the - # pre-selection draft behavior); the ready-for-review run is full. + # Untrusted selection skips the heavy matrices; the merge-queue + # run is full. fall_back_to_skip() { echo "mode=skip" >> "$GITHUB_OUTPUT" echo "core_files=" >> "$GITHUB_OUTPUT" @@ -648,14 +648,14 @@ jobs: fi if [[ "$SELECT_OUTCOME" != "success" ]] || [[ ! -s /tmp/selection.json ]]; then - echo "::warning::shadow selector did not produce output; draft skips heavy matrices (full run happens on ready for review)" + echo "::warning::shadow selector did not produce output; skipping heavy matrices (full run happens in the merge queue)" fall_back_to_skip selector_error exit 0 fi full_run_reasons=$(jq -r '.ast.full_run_reasons | length' /tmp/selection.json) if [[ "$full_run_reasons" -gt 0 ]]; then - echo "Selector requested a full run; draft defers it to ready for review:" + echo "Selector requested a full run; deferring it to the merge queue:" jq -r '.ast.full_run_reasons[]' /tmp/selection.json fall_back_to_skip full_run_requested exit 0 @@ -753,13 +753,15 @@ jobs: # Runs product tests in parallel — one matrix job per group # Each job gets its own runner + Docker stack, so no shared DB conflicts # Small products (< 50 tests) are grouped into a single job to avoid setup overhead + # Skipped on regular PRs — the merge queue's trunk-merge/** run is the full gate; + # the run-ci-backend label forces them on a PR. # A status function is required to inspect dependency results after failures. # Use !cancelled(), not always(), so superseded matrix jobs still receive cancellation. turbo-tests: needs: [changes, turbo-discover, detect-snapshot-mode, build-product-test-matrix, get_clickhouse_versions] if: >- !cancelled() && - (github.event.pull_request.draft != true || + (github.event_name != 'pull_request' || startsWith(github.head_ref, 'trunk-merge/') || contains(github.event.pull_request.labels.*.name, 'run-ci-backend')) && needs.turbo-discover.result == 'success' && @@ -2184,13 +2186,13 @@ jobs: RUN_POE: ${{ needs.select-tests.outputs.run_poe }} RUN_TEMPORAL: ${{ needs.select-tests.outputs.run_temporal }} CORE_FILES: ${{ needs.select-tests.outputs.core_files }} - # A trunk-merge/** PR opens as a draft but is the merge gate, and - # select-tests deliberately does not run for it. Reporting it as a - # draft here would turn that empty MODE into "skip" and build an - # empty matrix, so the queue would merge having run no Django tests - # at all. Not-draft makes the empty MODE fall through to the full - # matrix, which is what the queue has to run. - IS_DRAFT: ${{ github.event.pull_request.draft && !startsWith(github.head_ref, 'trunk-merge/') }} + # trunk-merge/** is excluded because select-tests deliberately does + # not run for it. Counting it as selective would turn that empty MODE + # into "skip" and build an empty matrix, so the queue would merge + # having run no Django tests at all. Leaving it non-selective makes + # the empty MODE fall through to the full matrix, which is what the + # queue has to run. + IS_SELECTIVE_PR: ${{ github.event_name == 'pull_request' && !startsWith(github.head_ref, 'trunk-merge/') }} FORCE_FULL: ${{ contains(github.event.pull_request.labels.*.name, 'run-ci-backend') }} # The new-events-schema variants double the matrix, so they only run on # PRs opted in via the test-new-events-schema label and on manual dispatch. @@ -2201,22 +2203,21 @@ jobs: # :NOTE: Keep shard counts/group ranges in sync with historical Django matrix tuning. # Consult #team-devex before changing. - # A draft PR must never fall back to the full matrix. select-tests only - # runs on drafts and sets MODE to "skip"/"selected"; an empty MODE on a - # draft means it was cancelled or failed (typically ready_for_review - # superseding it mid-flight). Without this, the draft builds the full - # matrix and holds the per-branch concurrency slot for ~30 min, so the - # ready-for-review run — the actual merge gate — queues behind it instead - # of starting. Skip here and defer to that ready run. The run-ci-backend - # label intentionally forces the full matrix on a draft, so honor it. - if [[ -z "$MODE" && "$IS_DRAFT" == "true" && "$FORCE_FULL" != "true" ]]; then - echo "::notice::Draft PR with no trusted Django selection — skipping heavy matrices; the ready-for-review run is the full gate" + # A regular PR must never fall back to the full matrix. select-tests + # only runs on regular PRs and sets MODE to "skip"/"selected"; an empty + # MODE on one means it was cancelled or failed (typically a newer push + # superseding it mid-flight). Without this, the PR builds the full + # matrix and holds the per-branch concurrency slot for ~30 min. Skip + # here and defer to the merge-queue run — the actual merge gate. The + # run-ci-backend label intentionally forces the full matrix, so honor it. + if [[ -z "$MODE" && "$IS_SELECTIVE_PR" == "true" && "$FORCE_FULL" != "true" ]]; then + echo "::notice::PR with no trusted Django selection — skipping heavy matrices; the merge-queue run is the full gate" MODE="skip" fi if [[ "$MODE" == "skip" ]]; then - # Draft PR where selection couldn't be trusted: skip the heavy - # matrices and defer to the full run on ready for review. + # PR where selection couldn't be trusted: skip the heavy + # matrices and defer to the full run in the merge queue. echo "include=[]" >> "$GITHUB_OUTPUT" echo "Django matrix size: 0 (mode=skip)" exit 0 @@ -2456,7 +2457,7 @@ jobs: # 1. Legacy code directly changed (ee/, posthog/) # 2. OR product changes affect legacy code (turbo-discover outputs run_legacy=true) # 3. OR turbo-discover itself failed (conservative: run Django on detection failure) - # The non-empty include guard covers drafts in skip mode and selected runs + # The non-empty include guard covers PRs in skip mode and selected runs # that chose zero files — an empty matrix would otherwise fail the job. # A status function is required to inspect dependency results after failures. # Use !cancelled(), not always(), so superseded matrix jobs still receive cancellation. @@ -3601,8 +3602,8 @@ jobs: needs: [select-tests] runs-on: ubuntu-latest timeout-minutes: 5 - # select-tests only runs on internal draft PRs; fire only when it actually - # produced a decision. DevEx project only, mirroring monitor-github-rate-limit; + # select-tests only runs on internal PRs outside the merge queue; fire only + # when it actually produced a decision. DevEx project only, mirroring monitor-github-rate-limit; # continue-on-error so telemetry never reds CI. Skipped on forks/Dependabot, # which never have the secret (and where select-tests didn't run anyway). if: | @@ -3717,15 +3718,17 @@ jobs: pull-requests: write # sticky CI report comment # In-repo PRs only — fork PRs get a read-only token that can't comment. Runs when either # the product (turbo-tests) or core (django_tests) suite ran, so a core-only PR still - # gets a comment; both skip on drafts, leaving nothing to report. - # drafts run a partial test selection — patch coverage would report false gaps + # gets a comment. + # Only label-forced full runs get a comment: regular PRs run a partial test + # selection — patch coverage would report false gaps — and the merge queue's + # trunk-merge/** PRs are ephemeral, so a comment there is never read. # Security invariant: this job runs PR-head-controlled scripts (coverage_report.py, # post-coverage-section.mjs) — never add secrets beyond github.token to this job # (report-test-timings checks out the base ref for exactly this reason). if: >- !cancelled() && github.event_name == 'pull_request' && - github.event.pull_request.draft == false && + contains(github.event.pull_request.labels.*.name, 'run-ci-backend') && github.event.pull_request.head.repo.full_name == 'PostHog/posthog' && (needs.turbo-tests.result != 'skipped' || needs.django_tests.result != 'skipped') steps: diff --git a/.github/workflows/ci-e2e-playwright.yml b/.github/workflows/ci-e2e-playwright.yml index 623e05cdc758..dac92f663192 100644 --- a/.github/workflows/ci-e2e-playwright.yml +++ b/.github/workflows/ci-e2e-playwright.yml @@ -8,8 +8,9 @@ on: pull_request: # Draft PRs skip E2E entirely — the broad path filter below matches nearly # every code PR, and a full ~25-minute stack boot per draft push is the - # remaining big per-PR cost. The ready-for-review full run is the merge - # gate; use workflow_dispatch to force a run on a draft. + # remaining big per-PR cost. Ready PRs run a narrowed spec selection; the + # full suite (the merge gate) runs on the merge queue's trunk-merge/** + # draft PRs. Use workflow_dispatch to force a run on a draft. types: [opened, synchronize, reopened, ready_for_review] workflow_dispatch: inputs: @@ -76,9 +77,10 @@ jobs: # keep ample headroom. A timeout here skips Playwright, which the aggregator # would count as a pass. timeout-minutes: 7 - # Run on master push, manual dispatch, and on ready internal-repo PRs. - # Drafts skip E2E (see the trigger comment); the `Playwright tests pass` - # aggregator treats skipped as success, and drafts can't merge anyway. + # Run on master push, manual dispatch, ready internal-repo PRs, and the + # merge queue's trunk-merge/** PRs (drafts, but the full merge gate). + # Other drafts skip E2E (see the trigger comment); the `Playwright tests + # pass` aggregator treats skipped as success, and drafts can't merge anyway. if: | github.event_name == 'push' || github.event_name == 'workflow_dispatch' || @@ -321,17 +323,17 @@ jobs: select-specs: name: Select affected E2E specs needs: [changes] - # Narrow the suite only on incremental `synchronize` pushes to a ready - # (non-draft) internal PR. Every other trigger — opened, reopened, - # ready_for_review, workflow_dispatch, and push to master — skips - # this job, leaving empty outputs so the playwright job runs the FULL suite. - # The merge gate (ready_for_review run) and the post-merge master run always - # run everything, so a selection miss is caught there. Fail-open is FULL, + # Narrow the suite on every run for a ready (non-draft) internal PR + # outside the merge queue. The merge queue's trunk-merge/** runs, + # workflow_dispatch, and pushes to master skip this job, leaving empty + # outputs so the playwright job runs the FULL suite. The merge gate + # (trunk-merge/** run) and the post-merge master run always run + # everything, so a selection miss is caught there. Fail-open is FULL, # never skip: over-selection would silently drop coverage. if: | needs.changes.outputs.shouldRun == 'true' && github.event_name == 'pull_request' - && github.event.action == 'synchronize' + && !startsWith(github.head_ref, 'trunk-merge/') && github.event.pull_request.draft != true && github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest diff --git a/.github/workflows/ci-frontend.yml b/.github/workflows/ci-frontend.yml index 6652da253140..8b1a6319a682 100644 --- a/.github/workflows/ci-frontend.yml +++ b/.github/workflows/ci-frontend.yml @@ -1,16 +1,17 @@ name: Frontend CI on: pull_request: - # Draft PRs run a narrowed jest selection; ready PRs run the full matrix - # (the merge gate). ready_for_review re-triggers the full run on the - # current head when a PR leaves draft. To force the full matrix on a - # draft, add the `run-ci-frontend` label. The `no-ci` label does the - # opposite: it silences this workflow on a draft entirely (prototypes). + # PRs (draft or ready) run a narrowed jest selection; the full matrix (the + # merge gate) runs on the merge queue's trunk-merge/** draft PRs, and the + # queue ejects the PR when it fails. To force the full matrix on a PR, add + # the `run-ci-frontend` label. The `no-ci` label does the opposite: it + # silences this workflow on a draft entirely (prototypes). # No labeled/unlabeled triggers: GitHub cannot filter a label trigger by # name, so every unrelated label re-ran the full jest matrix against a - # commit CI had already covered. A label here takes effect from the next - # push, or from ready_for_review. - types: [opened, synchronize, reopened, ready_for_review] + # commit CI had already covered. No ready_for_review either, now that + # draft and ready run the same thing. A label takes effect from the next + # push. + types: [opened, synchronize, reopened] push: branches: - master @@ -37,7 +38,8 @@ jobs: timeout-minutes: 5 # A draft PR labeled `no-ci` skips this workflow entirely (prototypes): # every other job needs this one, and the gates treat skipped as success. - # ready_for_review re-runs everything, so the merge gate is unaffected. + # The merge queue's trunk-merge/** draft PRs never carry the label, so the + # merge gate is unaffected. if: >- github.event_name != 'pull_request' || github.event.pull_request.draft != true @@ -171,14 +173,14 @@ jobs: token: ${{ steps.app-token.outputs.token || github.token }} filters: ${{ runner.temp }}/frontend-exclude.yml - # Pick which jest tests to run on draft PRs by passing changed files to + # Pick which jest tests to run on PRs by passing changed files to # `jest --findRelatedTests` (jest's resolver walks the import graph for us). - # Skipped on ready PRs and master pushes; the jest job falls back - # to its full FOSS×EE × chunk matrix in those cases. The ready-for-review - # full run is the merge gate. The run-ci-frontend label forces the full - # matrix on a draft. When selection can't be trusted on a draft (config - # change, oversized diff, selector failure), the draft skips jest entirely - # and defers to the ready full run. + # Skipped on the merge queue's trunk-merge/** PRs and master pushes; the jest + # job falls back to its full FOSS×EE × chunk matrix in those cases. The + # merge-queue full run is the merge gate. The run-ci-frontend label forces + # the full matrix on a PR. When selection can't be trusted (config change, + # oversized diff, selector failure), the PR skips jest entirely and defers + # to the merge-queue full run. select-jest-tests: name: Select jest tests runs-on: ubuntu-latest @@ -189,7 +191,6 @@ jobs: if: | !cancelled() && needs.changes.outputs.frontend_code == 'true' && github.event_name == 'pull_request' - && github.event.pull_request.draft == true && !startsWith(github.head_ref, 'trunk-merge/') && !contains(github.event.pull_request.labels.*.name, 'run-ci-frontend') outputs: @@ -223,12 +224,12 @@ jobs: run: | set -euo pipefail - # Untrusted selection on a draft skips jest entirely (should_run=false - # gates the jest job off); the ready-for-review full run is the gate. + # Untrusted selection skips jest entirely (should_run=false gates + # the jest job off); the merge-queue full run is the gate. # Use this whenever the selector can't reason about the diff — a # narrowed run it can't vouch for would be false confidence, and a - # full run on every draft push is the spend this selection avoids. - # The run-ci-frontend label forces the full matrix on a draft. + # full run on every PR push is the spend this selection avoids. + # The run-ci-frontend label forces the full matrix on a PR. fall_back_to_skip() { echo "mode=skip" >> "$GITHUB_OUTPUT" echo 'matrix={"include":[]}' >> "$GITHUB_OUTPUT" @@ -237,14 +238,14 @@ jobs: } if [[ "$DIFF_OUTCOME" != "success" ]] || [[ ! -s /tmp/changed.txt ]]; then - echo "::warning::could not derive changed files; draft skips jest (full run happens on ready for review)" + echo "::warning::could not derive changed files; skipping jest (full run happens in the merge queue)" fall_back_to_skip exit 0 fi changed_count=$(wc -l < /tmp/changed.txt) if [[ "$changed_count" -gt 200 ]]; then - echo "::notice::diff has $changed_count files (>200); draft skips jest (full run happens on ready for review)" + echo "::notice::diff has $changed_count files (>200); skipping jest (full run happens in the merge queue)" fall_back_to_skip exit 0 fi @@ -259,7 +260,7 @@ jobs: jest.*.ts|babel.config.js|tsconfig.json|tsconfig.*.json|\ package.json|pnpm-lock.yaml|webpack.config.js|\ .github/workflows/ci-frontend.yml|.nvmrc) - echo "::notice::config change ($f); draft skips jest (full run happens on ready for review)" + echo "::notice::config change ($f); skipping jest (full run happens in the merge queue)" fall_back_to_skip exit 0 ;; @@ -299,7 +300,7 @@ jobs: # Selective mode: single EE-tree job, no chunk sharding. The FOSS # structural check (ee/ removed) only matters when ee imports are - # touched; defer that to the ready-for-review full run. --findRelatedTests + # touched; defer that to the merge-queue full run. --findRelatedTests # walks jest's import graph from these files and runs the matching # *.test.* files. echo "mode=selective" >> "$GITHUB_OUTPUT" @@ -654,10 +655,10 @@ jobs: jest: runs-on: ubuntu-latest - # Draft PRs run the selective mode as a single unsharded job over every + # PRs run the selective mode as a single unsharded job over every # test reachable from the changed files. A PR touching a scene component # can pull in ~700 suites, which does not fit in 15 minutes. 20 covers it; - # the ready-for-review fanout shards well below either limit. + # the merge-queue fanout shards well below either limit. timeout-minutes: 20 needs: [changes, select-jest-tests] # A status function is required because select-jest-tests is skipped on ready @@ -670,11 +671,12 @@ jobs: strategy: # If one test fails, still run the others fail-fast: false - # On draft PRs, select-jest-tests may narrow this to a single EE job + # On PRs, select-jest-tests may narrow this to a single EE job # running only the tests reachable from changed frontend source files, # or skip jest entirely (should_run=false) when selection can't be - # trusted. On ready PRs and master pushes it's skipped, so - # we fall back to the full FOSS×EE × chunk fanout (the merge gate). + # trusted. On merge-queue trunk-merge/** PRs and master pushes it's + # skipped, so we fall back to the full FOSS×EE × chunk fanout (the + # merge gate). matrix: ${{ fromJson(needs.select-jest-tests.outputs.matrix || '{"segment":["FOSS","EE"],"chunk":[1,2,3,4]}') }} steps: @@ -721,7 +723,7 @@ jobs: # matches nothing), and CHANGED_FILES is already scoped to # frontend source files by select-jest-tests. --passWithNoTests # so a changed source file with no related test doesn't fail the - # run — the ready-for-review full matrix is the gate. + # run — the merge-queue full matrix is the gate. pnpm --filter=@posthog/frontend build:products # shellcheck disable=SC2086 pnpm --filter=@posthog/frontend exec jest \ diff --git a/.github/workflows/ci-mcp.yml b/.github/workflows/ci-mcp.yml index 31e2a8c1bdce..9658fe7ddd13 100644 --- a/.github/workflows/ci-mcp.yml +++ b/.github/workflows/ci-mcp.yml @@ -4,12 +4,13 @@ on: push: branches: [master] pull_request: - # Draft PRs run build + unit tests only; the integration tests (which boot - # the full PostHog backend and trigger on any non-pytest Python change) run - # when the PR is marked ready for review; that full run is the merge gate. + # PRs run build + unit tests only; the integration tests (which boot the + # full PostHog backend and trigger on any non-pytest Python change) run on + # the merge queue's trunk-merge/** draft PRs — that queue run is the merge + # gate. # The `no-ci` label silences this workflow on a draft entirely (prototypes). # No labeled/unlabeled triggers here, so it takes effect from the next push. - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, synchronize, reopened] concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} @@ -30,7 +31,8 @@ jobs: timeout-minutes: 5 # A draft PR labeled `no-ci` skips this workflow entirely (prototypes): # every other job needs this one, and the gates treat skipped as success. - # ready_for_review re-runs everything, so the merge gate is unaffected. + # The merge queue's trunk-merge/** draft PRs never carry the label, so the + # merge gate is unaffected. if: >- github.event_name != 'pull_request' || github.event.pull_request.draft != true @@ -274,11 +276,14 @@ jobs: runs-on: depot-ubuntu-24.04-4 needs: changes timeout-minutes: 30 - # Skipped on drafts: these boot the full PostHog backend and trigger on any - # non-pytest Python change, so they're the expensive half of this workflow. - # Build and unit tests still run on drafts; the `MCP Tests Pass` aggregator - # treats skipped as success, and the ready-for-review run is the merge gate. - if: needs.changes.outputs.mcp == 'true' && (github.event.pull_request.draft != true || startsWith(github.head_ref, 'trunk-merge/')) + # Skipped on regular PRs: these boot the full PostHog backend and trigger on + # any non-pytest Python change, so they're the expensive half of this + # workflow. Build and unit tests still run on PRs; the `MCP Tests Pass` + # aggregator treats skipped as success, and the merge queue's trunk-merge/** + # run is the merge gate. + if: | + needs.changes.outputs.mcp == 'true' && + (github.event_name != 'pull_request' || startsWith(github.head_ref, 'trunk-merge/')) env: DOCKERHUB_USERNAME: ${{ vars.DOCKERHUB_USER }} DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }} diff --git a/.github/workflows/ci-storybook.yml b/.github/workflows/ci-storybook.yml index a20c157e6a1d..4891563c00dd 100644 --- a/.github/workflows/ci-storybook.yml +++ b/.github/workflows/ci-storybook.yml @@ -1,16 +1,17 @@ name: Storybook on: pull_request: - # Draft PRs run a narrowed visual-regression matrix when the Storybook module - # graph can safely identify affected stories. Ready PRs run the full chromium - # matrix as the merge gate. To force the full matrix on a draft, add the - # `run-ci-frontend` label. The `no-ci` label does the opposite: it silences - # this workflow on a draft entirely (prototypes). + # PRs (draft or ready) run a narrowed visual-regression matrix when the + # Storybook module graph can safely identify affected stories. The full + # chromium matrix (the merge gate) runs on the merge queue's trunk-merge/** + # draft PRs. To force the full matrix on a PR, add the `run-ci-frontend` + # label. The `no-ci` label does the opposite: it silences this workflow on + # a draft entirely (prototypes). # No labeled/unlabeled triggers: GitHub cannot filter a label trigger by name, # so every unrelated label re-ran the full chromium matrix against a commit CI - # had already covered. A label here takes effect from the next push, or from - # ready_for_review. - types: [opened, synchronize, reopened, ready_for_review] + # had already covered. No ready_for_review either, now that draft and ready + # run the same thing. A label takes effect from the next push. + types: [opened, synchronize, reopened] push: branches: - master @@ -30,7 +31,8 @@ jobs: timeout-minutes: 5 # A draft PR labeled `no-ci` skips this workflow entirely (prototypes): # every other job needs this one, and the gates treat skipped as success. - # ready_for_review re-runs everything, so the merge gate is unaffected. + # The merge queue's trunk-merge/** draft PRs never carry the label, so the + # merge gate is unaffected. if: >- github.event_name != 'pull_request' || github.event.pull_request.draft != true @@ -772,8 +774,9 @@ jobs: needs: [select-stories] runs-on: ubuntu-latest timeout-minutes: 5 - # select-stories only runs on internal draft PRs; fire only when it produced a - # decision. DevEx project only; continue-on-error so telemetry never reds CI. + # select-stories only runs on internal PRs outside the merge queue; fire only + # when it produced a decision. DevEx project only; continue-on-error so + # telemetry never reds CI. if: | !cancelled() && github.run_attempt == '1' && github.actor != 'dependabot[bot]' @@ -803,11 +806,11 @@ jobs: "run_id": ${{ toJSON(github.run_id) }} } - # Pick a narrowed visual-regression matrix for draft PRs based on which stories - # the Vite module graph says are affected by the diff. Ready PRs and drafts with - # the run-ci-frontend label skip this job and use the full chromium matrix. - # Selector uncertainty skips draft visual regression because the ready run is - # still the merge gate. + # Pick a narrowed visual-regression matrix for PRs based on which stories + # the Vite module graph says are affected by the diff. The merge queue's + # trunk-merge/** PRs and PRs with the run-ci-frontend label skip this job + # and use the full chromium matrix. Selector uncertainty skips PR visual + # regression because the merge-queue run is the merge gate. select-stories: name: Story selection runs-on: ubuntu-latest @@ -818,7 +821,6 @@ jobs: if: | !cancelled() && needs.changes.outputs.frontend == 'true' && github.event_name == 'pull_request' - && github.event.pull_request.draft == true && !startsWith(github.head_ref, 'trunk-merge/') && !contains(github.event.pull_request.labels.*.name, 'run-ci-frontend') outputs: @@ -911,14 +913,14 @@ jobs: } if [[ "$SELECT_OUTCOME" != "success" ]] || [[ ! -s /tmp/selection-result.json ]]; then - echo "::warning::story selector did not produce output; skipping draft visual regression" + echo "::warning::story selector did not produce output; skipping PR visual regression" fall_back_to_skip selector_error exit 0 fi MODE=$(jq -r '.mode' /tmp/selection-result.json) if [[ "$MODE" != "selective" ]]; then - echo "Selector declined to narrow (mode=$MODE); skipping draft visual regression" + echo "Selector declined to narrow (mode=$MODE); skipping PR visual regression" fall_back_to_skip "$(jq -r '.reason // "full_requested"' /tmp/selection-result.json)" exit 0 fi @@ -930,7 +932,7 @@ jobs: echo "should_run=false" >> "$GITHUB_OUTPUT" echo "affected_files_json=[]" >> "$GITHUB_OUTPUT" emit_metrics true no_affected_stories - echo "Selective mode: 0 affected stories, skipping draft visual regression" + echo "Selective mode: 0 affected stories, skipping PR visual regression" exit 0 fi diff --git a/AGENTS.md b/AGENTS.md index 1626806b59c5..9c1b5156e51b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,10 +78,10 @@ NEVER share sensitive information in a PR description. Users may share sensitive Once a branch already has an open PR, push incremental changes and fixes to it without waiting for human guidance — keeping the PR current is part of the work. Pushes still trigger CI, which burns runner credits, so batch related commits and push once the increment is ready rather than after every change. -#### Forcing the full CI matrix on a draft +#### Forcing the full CI matrix on a PR -Draft PRs run a narrowed matrix. -The `run-ci-backend` and `run-ci-frontend` labels force the full one, but a label alone starts nothing: it takes effect on the next push, or when the PR is marked ready for review. +PRs run a narrowed matrix, draft or ready; the full one is the merge queue's job. +The `run-ci-backend` and `run-ci-frontend` labels force it on the PR itself, but a label alone starts nothing: it takes effect on the next push. An empty commit is enough. ```bash