From 9426ad25cd105887db86000886e49ff39363ced2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Mon, 27 Jul 2026 15:05:11 -0400 Subject: [PATCH 01/10] chore(ci): scope semgrep, mcp, playwright and nodejs path filters Gate the three semgrep jobs that had no `changes` filter at all: - semgrep-general only scans the repo root plus the directories the other semgrep jobs exclude, so a change confined to frontend/, posthog/ or products/ leaves it nothing to scan. - semgrep-devex mirrors the directory list it passes to semgrep. - semgrep-test-rules runs `semgrep --test .semgrep/`, which only exercises the rules against their own committed fixtures. Narrow semgrep-products-frontend to the scope prefer-codegen-api declares for itself (`/products/*/frontend/`, typescript/javascript, no generated). Exclude Django pytest modules from the MCP and Playwright filters. Both suites drive a running server and never import them. Deliberately scoped to pytest modules rather than whole test/ and tests/ directories: those hold helpers that runtime code does import, notably posthog/test/base.py via setup_test_environment and posthog/test/playwright_setup_functions.py which posthog/urls.py serves at api/setup_test/. Drop the '*Dockerfile' entry from the Node.js filter. It matched only the root Django Dockerfile, which no compose service builds, and never Dockerfile.node, which ci-nodejs-container.yml already gates. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .github/workflows/ci-e2e-playwright.yml | 14 +++++++ .github/workflows/ci-mcp.yml | 21 ++++++++++ .github/workflows/ci-nodejs.yml | 5 ++- .github/workflows/ci-security.yaml | 51 ++++++++++++++++++++++++- 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-e2e-playwright.yml b/.github/workflows/ci-e2e-playwright.yml index ab8ce97166cf..52fdcd2d91fd 100644 --- a/.github/workflows/ci-e2e-playwright.yml +++ b/.github/workflows/ci-e2e-playwright.yml @@ -152,6 +152,20 @@ jobs: # boot the whole stack. - '!**/*.md' - '!**/*.mdx' + # Same reasoning for Django's pytest modules: the suite drives the + # app through a browser and never loads them. Deliberately narrow: + # - pytest modules only. posthog/test/ also holds live server code + # (playwright_setup_functions.py, served via posthog/urls.py at + # api/setup_test/) which must keep triggering a run. + # - scoped to the Python trees, because an exclude vetoes the whole + # filter and tools/test_playwright_spec_selection.py above must + # keep forcing a full run. + - '!posthog/**/test_*.py' + - '!posthog/**/*_test.py' + - '!posthog/**/conftest.py' + - '!ee/**/test_*.py' + - '!ee/**/*_test.py' + - '!ee/**/conftest.py' # Visual Review auto-commits a snapshot baseline after human review and CI # approval. Re-running the ~25-min E2E stack for that bot push is pure waste; diff --git a/.github/workflows/ci-mcp.yml b/.github/workflows/ci-mcp.yml index dbe453001d70..72593df2d6d6 100644 --- a/.github/workflows/ci-mcp.yml +++ b/.github/workflows/ci-mcp.yml @@ -106,6 +106,27 @@ jobs: # CI config - '.github/workflows/ci-mcp.yml' - '.github/workflows/mcp-publish.yml' + # The integration tests drive a running Django server over HTTP, so + # a pytest module can't change the outcome, which means a PR touching + # only those can skip the ~15-30 min backend boot. Two narrowings: + # - pytest modules only, not whole test/ and tests/ directories. + # Those hold helpers that runtime code does import (posthog/ + # test/base.py via setup_test_environment, posthog/test/ + # playwright_setup_functions.py via posthog/urls.py). + # - scoped to the Django trees, because an exclude vetoes the + # whole filter and services/mcp/tests/** is this suite's own code. + - '!posthog/**/test_*.py' + - '!posthog/**/*_test.py' + - '!posthog/**/conftest.py' + - '!ee/**/test_*.py' + - '!ee/**/*_test.py' + - '!ee/**/conftest.py' + - '!common/**/test_*.py' + - '!common/**/*_test.py' + - '!common/**/conftest.py' + - '!products/*/backend/**/test_*.py' + - '!products/*/backend/**/*_test.py' + - '!products/*/backend/**/conftest.py' build: name: Build Package runs-on: ubuntu-latest diff --git a/.github/workflows/ci-nodejs.yml b/.github/workflows/ci-nodejs.yml index 6d34a1ae01e6..24e448a78a7b 100644 --- a/.github/workflows/ci-nodejs.yml +++ b/.github/workflows/ci-nodejs.yml @@ -68,8 +68,11 @@ jobs: - 'posthog/management/commands/setup_test_environment.py' - 'posthog/migrations/**' - 'posthog/plugins/**' + # Compose files only, deliberately no Dockerfile globs: these jobs run + # the suites from source via pnpm against the compose stack (whose + # only build contexts are rust/), so no app image is built here. + # Dockerfile.node is gated separately in ci-nodejs-container.yml. - 'docker*.yml' - - '*Dockerfile' rust: - 'rust/**' - 'proto/**' diff --git a/.github/workflows/ci-security.yaml b/.github/workflows/ci-security.yaml index a2fe30993fe8..e9e2afdc0d21 100644 --- a/.github/workflows/ci-security.yaml +++ b/.github/workflows/ci-security.yaml @@ -33,6 +33,9 @@ jobs: rust: ${{ steps.filter.outputs.rust || 'true' }} js: ${{ steps.filter.outputs.js || 'true' }} products-frontend: ${{ steps.filter.outputs.products-frontend || 'true' }} + general: ${{ steps.filter.outputs.general || 'true' }} + devex: ${{ steps.filter.outputs.devex || 'true' }} + test-rules: ${{ steps.filter.outputs.test-rules || 'true' }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -69,10 +72,50 @@ jobs: - 'services/stripe-app/**' - '.semgrep/rules/security/**' - '.github/workflows/ci-security.yaml' + # prefer-codegen-api is `languages: [typescript, javascript]` and + # scopes itself to `/products/*/frontend/` minus `**/generated/**`. + # Mirror that scope: a backend-only product change has nothing here + # to scan. products-frontend: - - 'products/**' + - 'products/*/frontend/**/*.{ts,tsx,js,jsx}' - '.semgrep/rules/security/prefer-codegen-api.yaml' - '.github/workflows/ci-security.yaml' + - '!**/generated/**' + # semgrep-general scans the repo root plus every directory the other + # jobs don't cover, so this list must stay in sync with the job's + # --exclude flags below. + general: + - '**' + - '!cli/**' + - '!common/**' + - '!ee/**' + - '!frontend/**' + - '!livestream/**' + - '!nodejs/**' + - '!posthog/**' + - '!products/**' + - '!rust/**' + - '!.semgrep/**' + - '!docs/**' + - '!services/**' + # Mirrors the directory list semgrep-devex passes to semgrep. Its + # rules span python/typescript/generic, so this can't narrow by + # extension. + devex: + - 'bin/**' + - 'common/**' + - 'ee/**' + - 'frontend/**' + - 'packages/**' + - 'posthog/**' + - 'products/**' + - '.semgrep/rules/devex/**' + - '.github/workflows/ci-security.yaml' + # `semgrep --test .semgrep/` only exercises the rules against their + # own committed fixtures, so nothing outside .semgrep/ can change it. + test-rules: + - '.semgrep/**' + - '.github/workflows/ci-security.yaml' semgrep-python: needs: changes @@ -274,6 +317,8 @@ jobs: # scans GitHub Actions and other repo-wide config semgrep-general: + needs: changes + if: needs.changes.outputs.general == 'true' runs-on: ubuntu-latest timeout-minutes: 20 @@ -332,6 +377,8 @@ jobs: # - ERROR-severity rules fail the job on any finding (regression guards). # See `.semgrep/rules/devex/README.md`. semgrep-devex: + needs: changes + if: needs.changes.outputs.devex == 'true' runs-on: ubuntu-latest timeout-minutes: 20 @@ -383,6 +430,8 @@ jobs: bin/ common/ ee/ frontend/ packages/ posthog/ products/ semgrep-test-rules: + needs: changes + if: needs.changes.outputs.test-rules == 'true' runs-on: ubuntu-latest timeout-minutes: 10 From b406c588a3e9788bb10b5bf37173f438c4e63ef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Mon, 27 Jul 2026 16:51:51 -0400 Subject: [PATCH 02/10] fix(ci): stop excluding production *_test.py files from mcp and e2e Review caught that `*_test.py` is not only a pytest convention under posthog/. Twenty-two production files use that suffix, including two ClickHouse migrations (0097_session_replay_events_v2_test.py, 0110_log_entries_v2_test.py), the DDL module they import, and the aggregate_funnel_*_test.py UDF scripts that MCP CI copies into ClickHouse before running `manage.py migrate_clickhouse`. Excluding the suffix meant a PR touching only those files skipped MCP CI and Playwright entirely, with both aggregators counting the skip as a pass. Drop the `*_test.py` exclude from the posthog/, ee/ and common/ trees and keep it only under products/*/backend, where it is the colocated-test convention and no production file uses it. Also drop the file-extension allowlist from the products-frontend filter. prefer-codegen-api is `languages: [typescript, javascript]`, which covers more suffixes than the allowlist kept up with, and products/dashboards/frontend/bin/generate-widget-config-zod.mjs already sat outside it. Scoping to the rule's own path bounds is enough. Refresh the comments the narrowing falsified: the ci-mcp.yml claims that the filter triggers on "any Python change across the app", the "language-specific" description of the semgrep changes job, and the fail-open NOTE, which does not hold for a truncated >3000-file PR under the include-'**' `general` filter. Correct the exclude-veto rationale in the new comments too: an exclude is tested against every file the filter sees, it does not veto the filter as a whole. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .github/workflows/ci-e2e-playwright.yml | 23 ++++++++------- .github/workflows/ci-mcp.yml | 38 ++++++++++++++----------- .github/workflows/ci-security.yaml | 31 +++++++++++++------- 3 files changed, 56 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci-e2e-playwright.yml b/.github/workflows/ci-e2e-playwright.yml index 52fdcd2d91fd..6016ac2a4e8c 100644 --- a/.github/workflows/ci-e2e-playwright.yml +++ b/.github/workflows/ci-e2e-playwright.yml @@ -152,19 +152,22 @@ jobs: # boot the whole stack. - '!**/*.md' - '!**/*.mdx' - # Same reasoning for Django's pytest modules: the suite drives the - # app through a browser and never loads them. Deliberately narrow: - # - pytest modules only. posthog/test/ also holds live server code - # (playwright_setup_functions.py, served via posthog/urls.py at - # api/setup_test/) which must keep triggering a run. - # - scoped to the Python trees, because an exclude vetoes the whole - # filter and tools/test_playwright_spec_selection.py above must - # keep forcing a full run. + # Same reasoning for Django's pytest modules: the suite drives the app + # through a browser and never loads them. Deliberately narrow: + # - pytest modules only. posthog/test/ also holds live server code, + # notably playwright_setup_functions.py which posthog/urls.py + # serves at api/setup_test/, so it must keep triggering a run. + # - the '*_test.py' suffix is not excluded, because under posthog/ it + # names production code (ClickHouse migrations, ClickHouse UDF + # scripts) rather than tests. + # - each exclude is prefixed with its tree, because an exclude is + # tested against every file the filter sees, not just the include + # it was written for. A bare '!**/test_*.py' would also veto + # tools/test_playwright_spec_selection.py above, which must keep + # forcing a full run. - '!posthog/**/test_*.py' - - '!posthog/**/*_test.py' - '!posthog/**/conftest.py' - '!ee/**/test_*.py' - - '!ee/**/*_test.py' - '!ee/**/conftest.py' # Visual Review auto-commits a snapshot baseline after human review and CI diff --git a/.github/workflows/ci-mcp.yml b/.github/workflows/ci-mcp.yml index 72593df2d6d6..19c336b9a3d7 100644 --- a/.github/workflows/ci-mcp.yml +++ b/.github/workflows/ci-mcp.yml @@ -5,8 +5,8 @@ on: branches: [master] pull_request: # Draft PRs run build + unit tests only; the integration tests (which boot - # the full PostHog backend and trigger on any Python change) run when the - # PR is marked ready for review — that full run is the merge gate. + # 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. # 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] @@ -40,8 +40,8 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 # MCP integration tests boot the full PostHog backend (Django web server, - # Celery worker, migrations, demo data), so the filter mirrors the Dagster - # approach: trigger on any Python change across the app. + # Celery worker, migrations, demo data), so the filter stays broad: any + # non-pytest Python change across the app triggers it. - uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1 id: app-token if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository @@ -106,23 +106,29 @@ jobs: # CI config - '.github/workflows/ci-mcp.yml' - '.github/workflows/mcp-publish.yml' - # The integration tests drive a running Django server over HTTP, so - # a pytest module can't change the outcome, which means a PR touching - # only those can skip the ~15-30 min backend boot. Two narrowings: + # The integration tests drive a running Django server over HTTP, so a + # pytest module can't change the outcome of any job this filter gates + # (build, unit tests, and the ~15-30 min backend boot). Three + # deliberate narrowings, each of which a wider pattern would break: # - pytest modules only, not whole test/ and tests/ directories. - # Those hold helpers that runtime code does import (posthog/ - # test/base.py via setup_test_environment, posthog/test/ - # playwright_setup_functions.py via posthog/urls.py). - # - scoped to the Django trees, because an exclude vetoes the - # whole filter and services/mcp/tests/** is this suite's own code. + # Those hold helpers runtime code imports, such as + # posthog/test/base.py via setup_test_environment and + # posthog/test/playwright_setup_functions.py via posthog/urls.py. + # - the '*_test.py' suffix is excluded only under products/*/backend, + # where it is the colocated-test convention. Under posthog/ it also + # names production code that this suite runs: ClickHouse migrations + # (0110_log_entries_v2_test.py), the DDL they import, and the + # aggregate_funnel_*_test.py UDF scripts copied into ClickHouse + # during setup. + # - each exclude is prefixed with its tree, because an exclude is + # tested against every file the filter sees, not just the include + # it was written for. A bare '!**/conftest.py' would also veto + # tools/hogli/tests/conftest.py, which tools/hogli/** includes. - '!posthog/**/test_*.py' - - '!posthog/**/*_test.py' - '!posthog/**/conftest.py' - '!ee/**/test_*.py' - - '!ee/**/*_test.py' - '!ee/**/conftest.py' - '!common/**/test_*.py' - - '!common/**/*_test.py' - '!common/**/conftest.py' - '!products/*/backend/**/test_*.py' - '!products/*/backend/**/*_test.py' @@ -271,7 +277,7 @@ jobs: needs: changes timeout-minutes: 30 # Skipped on drafts: these boot the full PostHog backend and trigger on any - # Python change, so they're the expensive half of this workflow. Build and + # 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 diff --git a/.github/workflows/ci-security.yaml b/.github/workflows/ci-security.yaml index e9e2afdc0d21..9bf3da958862 100644 --- a/.github/workflows/ci-security.yaml +++ b/.github/workflows/ci-security.yaml @@ -14,11 +14,17 @@ env: SEMGREP_IMAGE: semgrep/semgrep:1.167.0@sha256:06938c1f365d3f67b8cedd8bc117607ae64253f88a0e768e9da9408548927dd6 jobs: - # Job to decide if we should run language-specific semgrep scans. + # Job to decide which semgrep scans need to run. # See .github/actions/paths-filter/README.md for filter semantics # NOTE: With token auth, paths-filter uses pulls.listFiles which caps at # 3000 changed files. PRs exceeding that will silently truncate, but the # outputs default to 'true' so scans run unconditionally (safe fallback). + # That default only covers an absent output (filter step skipped or failed). + # The include-only filters below also truncate toward 'true', since any + # returned page will contain a file matching them. `general` is the + # exception: it includes '**' and is decided purely by its excludes, so a + # truncated >3000-file PR whose returned page sits entirely inside those + # excluded trees resolves to 'false'. changes: runs-on: ubuntu-latest timeout-minutes: 5 @@ -72,18 +78,22 @@ jobs: - 'services/stripe-app/**' - '.semgrep/rules/security/**' - '.github/workflows/ci-security.yaml' - # prefer-codegen-api is `languages: [typescript, javascript]` and - # scopes itself to `/products/*/frontend/` minus `**/generated/**`. - # Mirror that scope: a backend-only product change has nothing here - # to scan. + # prefer-codegen-api scopes itself to `/products/*/frontend/` minus + # `**/generated/**`, so mirror those two bounds and nothing more: a + # backend-only product change has nothing here to scan. Deliberately + # not narrowed by file extension, because the rule's `languages: + # [typescript, javascript]` covers more suffixes than an allowlist + # here would keep up with (.mjs and .cjs are already in use). products-frontend: - - 'products/*/frontend/**/*.{ts,tsx,js,jsx}' + - 'products/*/frontend/**' - '.semgrep/rules/security/prefer-codegen-api.yaml' - '.github/workflows/ci-security.yaml' - '!**/generated/**' - # semgrep-general scans the repo root plus every directory the other - # jobs don't cover, so this list must stay in sync with the job's - # --exclude flags below. + # semgrep-general scans everything its own --exclude flags don't drop, + # so these negations must stay one-for-one with those flags below. + # Dropping a flag there without dropping it here is the dangerous + # direction: the files come into scan scope while the job stops being + # triggered for them. general: - '**' - '!cli/**' @@ -337,7 +347,8 @@ jobs: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - # exclude all directories already scanned by other jobs + # exclude all directories already scanned by other jobs. + # Keep this list one-for-one with the `general` filter in the changes job. - name: Run Semgrep run: | docker run --rm -v "${{ github.workspace }}:/src" -w /src \ From 67556228087a3e8a8c319c81f4f2a6cc937df41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Mon, 27 Jul 2026 17:10:26 -0400 Subject: [PATCH 03/10] chore(ci): keep posthog_ai frontend out of the agent skills check `check-skills` boots Docker and Postgres, migrates, runs setup_dev, then builds the skills ZIP with `hogli build:skills`, whose affected-path map is `products/*/skills/*`. The filter's `products/posthog_ai/**` include drags in that product's React frontend, its largest subtree at ~204 files, which cannot change the ZIP. Measured on a PR that touched one file under products/posthog_ai/frontend: 23.9 job-minutes of Docker, Postgres and migrations to rebuild a ZIP from inputs the PR did not touch. Scoped to posthog_ai rather than the general `products/*/frontend/**` so a future deliberate frontend include in this filter is not silently vetoed, since an exclude is tested against every file the filter sees. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .github/workflows/ci-agent-skills.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/ci-agent-skills.yml b/.github/workflows/ci-agent-skills.yml index 07db4234dc54..596b1ba8cf0f 100644 --- a/.github/workflows/ci-agent-skills.yml +++ b/.github/workflows/ci-agent-skills.yml @@ -45,6 +45,14 @@ jobs: - 'products/posthog_ai/**' - '.github/workflows/ci-agent-skills.yml' - 'services/mcp/schema/**' + # check-skills boots Postgres, migrates, and runs + # `hogli build:skills`, which reads products/*/skills/* (see + # build.py's affected-path map). posthog_ai's React frontend is + # its largest subtree and cannot change that ZIP, so keep the + # broad product include above from dragging it in. Scoped to + # posthog_ai rather than 'products/*/frontend/**' so a future + # deliberate frontend include here isn't silently vetoed. + - '!products/posthog_ai/frontend/**' check-skills: name: Check agent skills From 47f983d0b9ef4e87c72b8fdf0dbafe5c84afbdb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Tue, 28 Jul 2026 09:39:04 -0400 Subject: [PATCH 04/10] chore(ci): leave the cross-cutting semgrep scans unconditional Reverts every change to ci-security.yaml, restoring semgrep-general, semgrep-devex and semgrep-test-rules to running on all PRs, and products-frontend to its original 'products/**' include. That file is owned by @PostHog/team-security, and keeping those three jobs unconditional is a decision the security team has taken twice: #68640 added path filtering for the language-specific shards while stating the three cross-cutting scans stay unconditional, and #70907 hit the same cost problem this PR was chasing and chose to give the general scan four workers instead, explicitly to speed it up "without weakening the check". Gating them on which paths changed is that team's call to make, not a side effect of a CI cost pass. The measurement stands on its own and belongs in a proposal to them rather than in this PR. What remains here are four scoping fixes in workflows owned by the teams that run them: the MCP and Playwright pytest excludes, the inverted '*Dockerfile' glob in the Node.js filter, and keeping the posthog_ai frontend out of the agent skills check. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .github/workflows/ci-security.yaml | 66 ++---------------------------- 1 file changed, 3 insertions(+), 63 deletions(-) diff --git a/.github/workflows/ci-security.yaml b/.github/workflows/ci-security.yaml index 9bf3da958862..a2fe30993fe8 100644 --- a/.github/workflows/ci-security.yaml +++ b/.github/workflows/ci-security.yaml @@ -14,17 +14,11 @@ env: SEMGREP_IMAGE: semgrep/semgrep:1.167.0@sha256:06938c1f365d3f67b8cedd8bc117607ae64253f88a0e768e9da9408548927dd6 jobs: - # Job to decide which semgrep scans need to run. + # Job to decide if we should run language-specific semgrep scans. # See .github/actions/paths-filter/README.md for filter semantics # NOTE: With token auth, paths-filter uses pulls.listFiles which caps at # 3000 changed files. PRs exceeding that will silently truncate, but the # outputs default to 'true' so scans run unconditionally (safe fallback). - # That default only covers an absent output (filter step skipped or failed). - # The include-only filters below also truncate toward 'true', since any - # returned page will contain a file matching them. `general` is the - # exception: it includes '**' and is decided purely by its excludes, so a - # truncated >3000-file PR whose returned page sits entirely inside those - # excluded trees resolves to 'false'. changes: runs-on: ubuntu-latest timeout-minutes: 5 @@ -39,9 +33,6 @@ jobs: rust: ${{ steps.filter.outputs.rust || 'true' }} js: ${{ steps.filter.outputs.js || 'true' }} products-frontend: ${{ steps.filter.outputs.products-frontend || 'true' }} - general: ${{ steps.filter.outputs.general || 'true' }} - devex: ${{ steps.filter.outputs.devex || 'true' }} - test-rules: ${{ steps.filter.outputs.test-rules || 'true' }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -78,53 +69,9 @@ jobs: - 'services/stripe-app/**' - '.semgrep/rules/security/**' - '.github/workflows/ci-security.yaml' - # prefer-codegen-api scopes itself to `/products/*/frontend/` minus - # `**/generated/**`, so mirror those two bounds and nothing more: a - # backend-only product change has nothing here to scan. Deliberately - # not narrowed by file extension, because the rule's `languages: - # [typescript, javascript]` covers more suffixes than an allowlist - # here would keep up with (.mjs and .cjs are already in use). products-frontend: - - 'products/*/frontend/**' - - '.semgrep/rules/security/prefer-codegen-api.yaml' - - '.github/workflows/ci-security.yaml' - - '!**/generated/**' - # semgrep-general scans everything its own --exclude flags don't drop, - # so these negations must stay one-for-one with those flags below. - # Dropping a flag there without dropping it here is the dangerous - # direction: the files come into scan scope while the job stops being - # triggered for them. - general: - - '**' - - '!cli/**' - - '!common/**' - - '!ee/**' - - '!frontend/**' - - '!livestream/**' - - '!nodejs/**' - - '!posthog/**' - - '!products/**' - - '!rust/**' - - '!.semgrep/**' - - '!docs/**' - - '!services/**' - # Mirrors the directory list semgrep-devex passes to semgrep. Its - # rules span python/typescript/generic, so this can't narrow by - # extension. - devex: - - 'bin/**' - - 'common/**' - - 'ee/**' - - 'frontend/**' - - 'packages/**' - - 'posthog/**' - 'products/**' - - '.semgrep/rules/devex/**' - - '.github/workflows/ci-security.yaml' - # `semgrep --test .semgrep/` only exercises the rules against their - # own committed fixtures, so nothing outside .semgrep/ can change it. - test-rules: - - '.semgrep/**' + - '.semgrep/rules/security/prefer-codegen-api.yaml' - '.github/workflows/ci-security.yaml' semgrep-python: @@ -327,8 +274,6 @@ jobs: # scans GitHub Actions and other repo-wide config semgrep-general: - needs: changes - if: needs.changes.outputs.general == 'true' runs-on: ubuntu-latest timeout-minutes: 20 @@ -347,8 +292,7 @@ jobs: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - # exclude all directories already scanned by other jobs. - # Keep this list one-for-one with the `general` filter in the changes job. + # exclude all directories already scanned by other jobs - name: Run Semgrep run: | docker run --rm -v "${{ github.workspace }}:/src" -w /src \ @@ -388,8 +332,6 @@ jobs: # - ERROR-severity rules fail the job on any finding (regression guards). # See `.semgrep/rules/devex/README.md`. semgrep-devex: - needs: changes - if: needs.changes.outputs.devex == 'true' runs-on: ubuntu-latest timeout-minutes: 20 @@ -441,8 +383,6 @@ jobs: bin/ common/ ee/ frontend/ packages/ posthog/ products/ semgrep-test-rules: - needs: changes - if: needs.changes.outputs.test-rules == 'true' runs-on: ubuntu-latest timeout-minutes: 10 From e06f9f59b0bf7330dff9a0e1d5979f5d0f9b82a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Tue, 28 Jul 2026 10:14:29 -0400 Subject: [PATCH 05/10] chore(ci): tighten the path filter comments The comments added earlier in this PR ran 7 to 18 lines each, against CLAUDE.md's "default to short or 1-line comments. Explain why, not what". Compressed them while keeping every fact that encodes a trap: - posthog/test/ holds code the running app imports, so it is not excludable wholesale. - '*_test.py' under posthog/ names production code, so that suffix is only excluded under products/*/backend. - an exclude is tested against every file the filter sees, hence per-tree prefixes rather than a bare '**/'. - ci-nodejs has no Dockerfile glob on purpose. - check-skills builds its ZIP from products/*/skills/*. Two other changes worth noting. ci-mcp.yml's runtime-import example is now posthog/test/base.py, which setup_test_environment imports during the backend boot this filter gates, rather than duplicating ci-e2e-playwright.yml's playwright_setup_functions.py; each file now carries the example relevant to it. And an em-dash on a line this PR already touched is gone, per the writing-code-comments skill. Comments only. Verified by parsing each workflow, re-parsing every `filters:` literal block (its comments are part of the string the action receives), and deep-comparing the result against the previous commit: byte-identical, so every match outcome is unchanged. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .github/workflows/ci-agent-skills.yml | 12 ++++---- .github/workflows/ci-e2e-playwright.yml | 24 +++++++-------- .github/workflows/ci-mcp.yml | 39 +++++++++++-------------- .github/workflows/ci-nodejs.yml | 8 ++--- 4 files changed, 37 insertions(+), 46 deletions(-) diff --git a/.github/workflows/ci-agent-skills.yml b/.github/workflows/ci-agent-skills.yml index 596b1ba8cf0f..cb0d1fdaaab5 100644 --- a/.github/workflows/ci-agent-skills.yml +++ b/.github/workflows/ci-agent-skills.yml @@ -45,13 +45,11 @@ jobs: - 'products/posthog_ai/**' - '.github/workflows/ci-agent-skills.yml' - 'services/mcp/schema/**' - # check-skills boots Postgres, migrates, and runs - # `hogli build:skills`, which reads products/*/skills/* (see - # build.py's affected-path map). posthog_ai's React frontend is - # its largest subtree and cannot change that ZIP, so keep the - # broad product include above from dragging it in. Scoped to - # posthog_ai rather than 'products/*/frontend/**' so a future - # deliberate frontend include here isn't silently vetoed. + # check-skills zips products/*/skills/* via `hogli build:skills` + # (see build.py's affected-path map), so posthog_ai's frontend + # can't affect it. Scoped to posthog_ai, not + # 'products/*/frontend/**', so a future deliberate frontend + # include here isn't silently vetoed. - '!products/posthog_ai/frontend/**' check-skills: diff --git a/.github/workflows/ci-e2e-playwright.yml b/.github/workflows/ci-e2e-playwright.yml index 6016ac2a4e8c..72b7c4dde08c 100644 --- a/.github/workflows/ci-e2e-playwright.yml +++ b/.github/workflows/ci-e2e-playwright.yml @@ -152,19 +152,17 @@ jobs: # boot the whole stack. - '!**/*.md' - '!**/*.mdx' - # Same reasoning for Django's pytest modules: the suite drives the app - # through a browser and never loads them. Deliberately narrow: - # - pytest modules only. posthog/test/ also holds live server code, - # notably playwright_setup_functions.py which posthog/urls.py - # serves at api/setup_test/, so it must keep triggering a run. - # - the '*_test.py' suffix is not excluded, because under posthog/ it - # names production code (ClickHouse migrations, ClickHouse UDF - # scripts) rather than tests. - # - each exclude is prefixed with its tree, because an exclude is - # tested against every file the filter sees, not just the include - # it was written for. A bare '!**/test_*.py' would also veto - # tools/test_playwright_spec_selection.py above, which must keep - # forcing a full run. + # Same for Django's pytest modules: the suite drives the app through a + # browser and never loads them. Narrow on purpose: + # - modules, not whole test/ dirs: posthog/test/ also holds live + # server code, notably playwright_setup_functions.py, which + # posthog/urls.py serves at api/setup_test/. + # - no '*_test.py': under posthog/ that suffix names production code + # (ClickHouse migrations, ClickHouse UDF scripts), not tests. + # - per-tree prefixes, because an exclude is tested against every file + # the filter sees, not just the include it sits next to: a bare + # '!**/test_*.py' would also veto the include above for + # tools/test_playwright_spec_selection.py. - '!posthog/**/test_*.py' - '!posthog/**/conftest.py' - '!ee/**/test_*.py' diff --git a/.github/workflows/ci-mcp.yml b/.github/workflows/ci-mcp.yml index 19c336b9a3d7..2044962ec6ca 100644 --- a/.github/workflows/ci-mcp.yml +++ b/.github/workflows/ci-mcp.yml @@ -6,7 +6,7 @@ on: 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. + # when the PR is marked ready for review; that full 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] @@ -106,24 +106,19 @@ jobs: # CI config - '.github/workflows/ci-mcp.yml' - '.github/workflows/mcp-publish.yml' - # The integration tests drive a running Django server over HTTP, so a - # pytest module can't change the outcome of any job this filter gates - # (build, unit tests, and the ~15-30 min backend boot). Three - # deliberate narrowings, each of which a wider pattern would break: - # - pytest modules only, not whole test/ and tests/ directories. - # Those hold helpers runtime code imports, such as - # posthog/test/base.py via setup_test_environment and - # posthog/test/playwright_setup_functions.py via posthog/urls.py. - # - the '*_test.py' suffix is excluded only under products/*/backend, - # where it is the colocated-test convention. Under posthog/ it also - # names production code that this suite runs: ClickHouse migrations - # (0110_log_entries_v2_test.py), the DDL they import, and the - # aggregate_funnel_*_test.py UDF scripts copied into ClickHouse - # during setup. - # - each exclude is prefixed with its tree, because an exclude is - # tested against every file the filter sees, not just the include - # it was written for. A bare '!**/conftest.py' would also veto - # tools/hogli/tests/conftest.py, which tools/hogli/** includes. + # The integration tests drive a running Django server over HTTP and + # never import pytest modules, so exclude them. Narrow on purpose: + # - modules, not whole test/ dirs: those also hold runtime code, such + # as posthog/test/base.py, which the backend boot imports via + # setup_test_environment. + # - '*_test.py' only under products/*/backend, where it is the + # colocated-test convention. Under posthog/ that suffix names + # production code this suite runs (ClickHouse migrations, UDF + # scripts copied into ClickHouse during setup). + # - per-tree prefixes, because an exclude is tested against every file + # the filter sees, not just the include it sits next to: a bare + # '!**/conftest.py' would also veto tools/hogli/tests/conftest.py, + # which the tools/hogli/** include above covers. - '!posthog/**/test_*.py' - '!posthog/**/conftest.py' - '!ee/**/test_*.py' @@ -277,9 +272,9 @@ jobs: 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. + # 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 env: DOCKERHUB_USERNAME: ${{ vars.DOCKERHUB_USER }} diff --git a/.github/workflows/ci-nodejs.yml b/.github/workflows/ci-nodejs.yml index 24e448a78a7b..78fa89747eee 100644 --- a/.github/workflows/ci-nodejs.yml +++ b/.github/workflows/ci-nodejs.yml @@ -68,10 +68,10 @@ jobs: - 'posthog/management/commands/setup_test_environment.py' - 'posthog/migrations/**' - 'posthog/plugins/**' - # Compose files only, deliberately no Dockerfile globs: these jobs run - # the suites from source via pnpm against the compose stack (whose - # only build contexts are rust/), so no app image is built here. - # Dockerfile.node is gated separately in ci-nodejs-container.yml. + # Compose files only, no Dockerfile glob: these jobs run the suites from + # source via pnpm and the compose stack's only build contexts are rust/, + # so no app image is built here. Dockerfile.node is gated separately in + # ci-nodejs-container.yml. - 'docker*.yml' rust: - 'rust/**' From 640173efe7cf3789d2af82bb352e95f1c5c42e0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Tue, 28 Jul 2026 10:37:41 -0400 Subject: [PATCH 06/10] perf(ci): stop unrelated labels re-running the merge gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ci-backend, ci-frontend, ci-storybook, ci-ai and ci-hobby all take `labeled` and `unlabeled` in their pull_request types. Their comments say why: the run-ci-*, no-ci, evals-ready and hobby-preview escape hatches have to take effect without needing a push. But the trigger fires on *every* label, and only those are read. stamphog[bot] applies its approval stamp to nearly every PR (31 of 40 sampled) and often removes it again. Each add and each remove re-dispatches the full gating suite against a SHA that already went green. PR #74204: `labeled stamphog` at 12:36:54, three runs dispatched at 12:36:58 with identical matrices (23/15/6 jobs, same names) — 165 job-minutes for a label. PR #73987 is worse, because the removal dispatches too: 16:59:45 Storybook + Frontend + Backend -> all success 17:40:10 labeled stamphog -> 3 runs at 17:40:13 17:43:02 unlabeled stamphog -> 3 more at 17:43:05, cancelling those Measured across ten sampled PRs: 318 duplicate successful gating job-minutes, 285 of it on frontend PRs where Storybook (~110 min) and Frontend CI (~52 min) re-run. 22 of 30 sampled frontend PRs affected. Each workflow now ignores label events for labels it does not read. Every other trigger is untouched, so opened / synchronize / ready_for_review / reopened / master push all behave exactly as before and the merge gate is unchanged. The label lists were read out of each file rather than assumed: ci-backend reads four labels, not two. This follows the existing pattern in review-hog.yml, which already gates on github.event.label.name, and reuses the skip path the `no-ci` guard on the same expression already relies on: every other job needs `changes`, and the gates treat skipped as success. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .github/workflows/ci-ai.yml | 15 ++++++++++++--- .github/workflows/ci-backend.yml | 17 +++++++++++++++-- .github/workflows/ci-frontend.yml | 17 +++++++++++++++-- .github/workflows/ci-hobby.yml | 11 ++++++++++- .github/workflows/ci-storybook.yml | 17 +++++++++++++++-- 5 files changed, 67 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci-ai.yml b/.github/workflows/ci-ai.yml index 85998ea78340..60f3f2beb99a 100644 --- a/.github/workflows/ci-ai.yml +++ b/.github/workflows/ci-ai.yml @@ -18,9 +18,18 @@ jobs: env: DOCKERHUB_USERNAME: ${{ vars.DOCKERHUB_USER }} DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }} - # Skipping on forks as Braintrust credentials are not available there - if: | - github.repository == 'PostHog/posthog' && contains(github.event.pull_request.labels.*.name, 'evals-ready') + # Skipping on forks as Braintrust credentials are not available there. + # `evals-ready` is the only label this workflow reads, and the + # labeled/unlabeled triggers exist for it alone, so ignore every other label: + # once evals-ready is on a PR, an unrelated label would otherwise re-dispatch + # the whole 45-minute eval matrix against an unchanged SHA. + if: >- + github.repository == 'PostHog/posthog' + && contains(github.event.pull_request.labels.*.name, 'evals-ready') + && ( + !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) + || github.event.label.name == 'evals-ready' + ) strategy: fail-fast: false diff --git a/.github/workflows/ci-backend.yml b/.github/workflows/ci-backend.yml index 60279ee3c3e0..354d11f16893 100644 --- a/.github/workflows/ci-backend.yml +++ b/.github/workflows/ci-backend.yml @@ -84,10 +84,23 @@ jobs: # 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 labeled/unlabeled triggers exist only for the four labels this workflow + # reads, so every other label is ignored here. Without this, any unrelated + # label re-dispatches the full Django and product matrices against a SHA that + # already went green, and both adding and removing one costs a full run. if: >- github.event_name != 'pull_request' - || github.event.pull_request.draft != true - || !contains(github.event.pull_request.labels.*.name, 'no-ci') + || ( + ( + !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) + || contains(fromJSON('["no-ci", "no-snapshot-update", "run-ci-backend", "test-new-events-schema"]'), github.event.label.name) + ) + && ( + github.event.pull_request.draft != true + || !contains(github.event.pull_request.labels.*.name, 'no-ci') + ) + ) name: Determine need to run backend and migration checks # Set job outputs to values from filter step outputs: diff --git a/.github/workflows/ci-frontend.yml b/.github/workflows/ci-frontend.yml index f314b8ce2978..392331146f49 100644 --- a/.github/workflows/ci-frontend.yml +++ b/.github/workflows/ci-frontend.yml @@ -36,10 +36,23 @@ jobs: # 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 labeled/unlabeled triggers exist only for the two labels this workflow + # reads, so every other label is ignored here. Without this, any unrelated + # label re-dispatches the full jest matrix against a SHA that already went + # green, and both adding and removing one costs a full run. if: >- github.event_name != 'pull_request' - || github.event.pull_request.draft != true - || !contains(github.event.pull_request.labels.*.name, 'no-ci') + || ( + ( + !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) + || contains(fromJSON('["no-ci", "run-ci-frontend"]'), github.event.label.name) + ) + && ( + github.event.pull_request.draft != true + || !contains(github.event.pull_request.labels.*.name, 'no-ci') + ) + ) name: Determine need to run frontend checks # A snapshot-only bot push (Visual Review auto-commits frontend/snapshots.yml after # approval) skips ci-storybook and ci-e2e-playwright (the snapshot-baseline-dependent diff --git a/.github/workflows/ci-hobby.yml b/.github/workflows/ci-hobby.yml index 3fd45c4782b9..931449a01844 100644 --- a/.github/workflows/ci-hobby.yml +++ b/.github/workflows/ci-hobby.yml @@ -29,7 +29,16 @@ jobs: changes: runs-on: ubuntu-latest timeout-minutes: 5 - if: github.event_name == 'pull_request' + # `hobby-preview` is the only label this workflow reads (on add to create the + # droplet, on remove to tear it down), and the labeled/unlabeled triggers exist + # for it alone. Ignore every other label so an unrelated one doesn't re-dispatch + # the droplet smoke test against a SHA that already ran it. + if: >- + github.event_name == 'pull_request' + && ( + !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) + || github.event.label.name == 'hobby-preview' + ) name: Determine need to run hobby checks outputs: should_run: ${{ steps.filter.outputs.hobby == 'true' || steps.check-label.outputs.has_label == 'true' }} diff --git a/.github/workflows/ci-storybook.yml b/.github/workflows/ci-storybook.yml index 5aacc226919c..060a21517160 100644 --- a/.github/workflows/ci-storybook.yml +++ b/.github/workflows/ci-storybook.yml @@ -29,10 +29,23 @@ jobs: # 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 labeled/unlabeled triggers exist only for the two labels this workflow + # reads, so every other label is ignored here. Without this, any unrelated + # label re-dispatches the whole chromium matrix against a SHA that already + # went green, and both adding and removing one costs a full run. if: >- github.event_name != 'pull_request' - || github.event.pull_request.draft != true - || !contains(github.event.pull_request.labels.*.name, 'no-ci') + || ( + ( + !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) + || contains(fromJSON('["no-ci", "run-ci-frontend"]'), github.event.label.name) + ) + && ( + github.event.pull_request.draft != true + || !contains(github.event.pull_request.labels.*.name, 'no-ci') + ) + ) name: Determine need to run storybook checks # Set job outputs to values from filter step outputs: From 14dd21ea862796d021d217be09477b0e59b31cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Tue, 28 Jul 2026 11:08:57 -0400 Subject: [PATCH 07/10] revert(ci): drop the label-event guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts 640173e. This approach was already tried and rejected in #65900, and the version I pushed had the P1 hole that review caught. The hole, quoted from the Codex review on the same ci-backend.yml line I edited: with pull_request still firing for every labeled/unlabeled event, a job-level guard on `changes` means an unrelated label starts a run where `changes` is skipped, every downstream job cascades to skipped, and the required aggregator runs with `if: always()` treating skipped as success. So an unrelated label can replace a failed or in-progress `Django Tests Pass` with a green one without running any tests — and the workflow-level `cancel-in-progress` kills the legitimate run on the way. Verified present in what I pushed: `django_tests`, `frontend_tests` and `visual_regression_tests` all run `if: always()` and pass on `result == 'skipped'`, and the concurrency group is keyed on head_ref alone, so it does not separate label events from real ones. A false green on a required merge check is strictly worse than the runner time it saves. #65900 also patched that hole (separate `-noop` concurrency group plus the same label filter on each aggregator) and was still rejected, because the label list then has to be repeated in three places per workflow across nine files. The maintainer's verdict was "Given the downsides, I don't like any of the approaches here." The waste is real and still unaddressed — one review request costs two full gate re-dispatches. But the fix needs a design that doesn't distribute the label list, e.g. dropping labeled/unlabeled from the triggers in favour of a single dispatcher. That belongs in its own proposal, not here. Leaves this PR as the four path-filter fixes it started as. Also clears the ci-backend-shadow-drift failure, since ci-backend.yml is no longer touched and so needs no matching .depot/workflows/ci-backend.yml bump. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .github/workflows/ci-ai.yml | 15 +++------------ .github/workflows/ci-backend.yml | 17 ++--------------- .github/workflows/ci-frontend.yml | 17 ++--------------- .github/workflows/ci-hobby.yml | 11 +---------- .github/workflows/ci-storybook.yml | 17 ++--------------- 5 files changed, 10 insertions(+), 67 deletions(-) diff --git a/.github/workflows/ci-ai.yml b/.github/workflows/ci-ai.yml index 60f3f2beb99a..85998ea78340 100644 --- a/.github/workflows/ci-ai.yml +++ b/.github/workflows/ci-ai.yml @@ -18,18 +18,9 @@ jobs: env: DOCKERHUB_USERNAME: ${{ vars.DOCKERHUB_USER }} DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }} - # Skipping on forks as Braintrust credentials are not available there. - # `evals-ready` is the only label this workflow reads, and the - # labeled/unlabeled triggers exist for it alone, so ignore every other label: - # once evals-ready is on a PR, an unrelated label would otherwise re-dispatch - # the whole 45-minute eval matrix against an unchanged SHA. - if: >- - github.repository == 'PostHog/posthog' - && contains(github.event.pull_request.labels.*.name, 'evals-ready') - && ( - !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) - || github.event.label.name == 'evals-ready' - ) + # Skipping on forks as Braintrust credentials are not available there + if: | + github.repository == 'PostHog/posthog' && contains(github.event.pull_request.labels.*.name, 'evals-ready') strategy: fail-fast: false diff --git a/.github/workflows/ci-backend.yml b/.github/workflows/ci-backend.yml index 354d11f16893..60279ee3c3e0 100644 --- a/.github/workflows/ci-backend.yml +++ b/.github/workflows/ci-backend.yml @@ -84,23 +84,10 @@ jobs: # 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 labeled/unlabeled triggers exist only for the four labels this workflow - # reads, so every other label is ignored here. Without this, any unrelated - # label re-dispatches the full Django and product matrices against a SHA that - # already went green, and both adding and removing one costs a full run. if: >- github.event_name != 'pull_request' - || ( - ( - !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) - || contains(fromJSON('["no-ci", "no-snapshot-update", "run-ci-backend", "test-new-events-schema"]'), github.event.label.name) - ) - && ( - github.event.pull_request.draft != true - || !contains(github.event.pull_request.labels.*.name, 'no-ci') - ) - ) + || github.event.pull_request.draft != true + || !contains(github.event.pull_request.labels.*.name, 'no-ci') name: Determine need to run backend and migration checks # Set job outputs to values from filter step outputs: diff --git a/.github/workflows/ci-frontend.yml b/.github/workflows/ci-frontend.yml index 392331146f49..f314b8ce2978 100644 --- a/.github/workflows/ci-frontend.yml +++ b/.github/workflows/ci-frontend.yml @@ -36,23 +36,10 @@ jobs: # 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 labeled/unlabeled triggers exist only for the two labels this workflow - # reads, so every other label is ignored here. Without this, any unrelated - # label re-dispatches the full jest matrix against a SHA that already went - # green, and both adding and removing one costs a full run. if: >- github.event_name != 'pull_request' - || ( - ( - !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) - || contains(fromJSON('["no-ci", "run-ci-frontend"]'), github.event.label.name) - ) - && ( - github.event.pull_request.draft != true - || !contains(github.event.pull_request.labels.*.name, 'no-ci') - ) - ) + || github.event.pull_request.draft != true + || !contains(github.event.pull_request.labels.*.name, 'no-ci') name: Determine need to run frontend checks # A snapshot-only bot push (Visual Review auto-commits frontend/snapshots.yml after # approval) skips ci-storybook and ci-e2e-playwright (the snapshot-baseline-dependent diff --git a/.github/workflows/ci-hobby.yml b/.github/workflows/ci-hobby.yml index 931449a01844..3fd45c4782b9 100644 --- a/.github/workflows/ci-hobby.yml +++ b/.github/workflows/ci-hobby.yml @@ -29,16 +29,7 @@ jobs: changes: runs-on: ubuntu-latest timeout-minutes: 5 - # `hobby-preview` is the only label this workflow reads (on add to create the - # droplet, on remove to tear it down), and the labeled/unlabeled triggers exist - # for it alone. Ignore every other label so an unrelated one doesn't re-dispatch - # the droplet smoke test against a SHA that already ran it. - if: >- - github.event_name == 'pull_request' - && ( - !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) - || github.event.label.name == 'hobby-preview' - ) + if: github.event_name == 'pull_request' name: Determine need to run hobby checks outputs: should_run: ${{ steps.filter.outputs.hobby == 'true' || steps.check-label.outputs.has_label == 'true' }} diff --git a/.github/workflows/ci-storybook.yml b/.github/workflows/ci-storybook.yml index 060a21517160..5aacc226919c 100644 --- a/.github/workflows/ci-storybook.yml +++ b/.github/workflows/ci-storybook.yml @@ -29,23 +29,10 @@ jobs: # 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 labeled/unlabeled triggers exist only for the two labels this workflow - # reads, so every other label is ignored here. Without this, any unrelated - # label re-dispatches the whole chromium matrix against a SHA that already - # went green, and both adding and removing one costs a full run. if: >- github.event_name != 'pull_request' - || ( - ( - !contains(fromJSON('["labeled", "unlabeled"]'), github.event.action) - || contains(fromJSON('["no-ci", "run-ci-frontend"]'), github.event.label.name) - ) - && ( - github.event.pull_request.draft != true - || !contains(github.event.pull_request.labels.*.name, 'no-ci') - ) - ) + || github.event.pull_request.draft != true + || !contains(github.event.pull_request.labels.*.name, 'no-ci') name: Determine need to run storybook checks # Set job outputs to values from filter step outputs: From 17a79dc38e4963a6f9a84861cdfe1df2b8ea23a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n-Otero?= Date: Tue, 28 Jul 2026 13:48:21 -0400 Subject: [PATCH 08/10] perf(ci): drop the label triggers from the three merge gates Backend CI, Frontend CI and Storybook take `labeled`/`unlabeled` in their pull_request types. GitHub cannot filter a label trigger by name, so every label re-dispatches all three against a commit they have already covered. Measured: one `stamphog` label on #74204 dispatched 13 workflow runs totalling 168 job-minutes. 165 of those were these three; the other ten workflows gate on their own label name and skip for ~1.3 job-minutes combined, PR Approval Agent included, which spent 1.9 doing the job the label was actually added for. On a Python PR the Backend CI half is far worse: #73694, #73742 and #74032 each ran the full 61-shard Django matrix twice on one SHA, ~530 job-minutes a time, job fingerprints identical, one label event and no ready_for_review between them. Usage since 2026-07-01 says the trigger does not earn that: labels these workflows actually read were applied 124 times (run-ci-frontend 116, run-ci-backend 7, no-ci 1), against 1,774+ events from labels they do not read. Deleting the trigger rather than filtering inside the workflow, because filtering is unsafe here. The `changes` job gates everything else, and the required aggregators run `if: always()` and pass on a skipped dependency, so a guard that skips `changes` makes them post green without testing while `cancel-in-progress` kills the real run. #73160 uses that same skip path deliberately but bounds it to drafts, which cannot merge. Label events are not so bounded: pr-approval-agent excludes drafts, so stamphog only lands on ready PRs, which is exactly where a false green merges. #65900 tried the in-workflow guard across nine workflows, hit that P1 in review, patched it with a `-noop` concurrency group plus per-aggregator guards, and was closed as unmaintainable. `ci-mcp.yml` already runs without these triggers and documents the trade: a label takes effect from the next push. The comments here now say the same. Every label read is unchanged, so run-ci-backend, run-ci-frontend, no-ci, no-snapshot-update and test-new-events-schema all still work; they apply on the next push or on ready_for_review. Left alone deliberately: ci-hobby, whose droplet teardown fires on the `unlabeled` event, so removing its trigger would leak droplets; ci-ai, where `evals-ready` is the sole activation; and the six others that already gate correctly. Adding `workflow_dispatch` to ci-frontend and ci-storybook as a push-free fallback was considered and dropped, since a dispatch run has no pull_request context for the paths filter or the draft checks. The depot shadow moves in lockstep, as ci-backend-shadow-drift requires. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731 --- .depot/workflows/ci-backend.yml | 9 +++++---- .github/workflows/ci-backend.yml | 12 +++++++----- .github/workflows/ci-frontend.yml | 12 +++++++----- .github/workflows/ci-storybook.yml | 12 +++++++----- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/.depot/workflows/ci-backend.yml b/.depot/workflows/ci-backend.yml index df9781c6be02..edb9b6bc96c6 100644 --- a/.depot/workflows/ci-backend.yml +++ b/.depot/workflows/ci-backend.yml @@ -93,10 +93,11 @@ on: # 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 (labeled/unlabeled re-trigger) to force the full - # matrices on a draft; the `no-ci` label silences the workflow on a draft - # entirely. Mirrors canonical. - types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] + # `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] concurrency: # PRs: one active run per branch, cancel stale. Push: per-SHA so master # pushes never cancel each other (check-migrations always completes). diff --git a/.github/workflows/ci-backend.yml b/.github/workflows/ci-backend.yml index 60279ee3c3e0..edbaad8965ef 100644 --- a/.github/workflows/ci-backend.yml +++ b/.github/workflows/ci-backend.yml @@ -17,15 +17,17 @@ on: # 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 — labeled/unlabeled re-trigger the run so the matrices - # start without needing a new push. Cheap checks still run on drafts. + # `run-ci-backend` label. Cheap checks still run on drafts. # The `no-ci` label does the opposite: it silences this workflow on a draft - # entirely (prototypes/spikes), until the label is removed or the PR is - # marked ready for review. + # 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. # 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, labeled, unlabeled] + types: [opened, synchronize, reopened, ready_for_review] concurrency: # PRs: one active run per branch, cancel stale. Push: per-SHA so master diff --git a/.github/workflows/ci-frontend.yml b/.github/workflows/ci-frontend.yml index f314b8ce2978..7ae1b6229fdf 100644 --- a/.github/workflows/ci-frontend.yml +++ b/.github/workflows/ci-frontend.yml @@ -4,11 +4,13 @@ on: # 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 — labeled/unlabeled re-trigger - # the run so it starts without needing a new push. The `no-ci` label does - # the opposite: it silences this workflow on a draft entirely (prototypes), - # until the label is removed or the PR is marked ready for review. - types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] + # draft, 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] push: branches: - master diff --git a/.github/workflows/ci-storybook.yml b/.github/workflows/ci-storybook.yml index 5aacc226919c..01d0a11b0b34 100644 --- a/.github/workflows/ci-storybook.yml +++ b/.github/workflows/ci-storybook.yml @@ -4,11 +4,13 @@ on: # 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 — - # labeled/unlabeled re-trigger the run so it starts without needing a push. - # The `no-ci` label does the opposite: it silences this workflow on a draft - # entirely (prototypes), until it is removed or the PR is marked ready. - types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] + # `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] push: branches: - master From 1edd0ce4cd086cda467f6919fcc8417bbe6f3bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Tue, 28 Jul 2026 14:31:43 -0400 Subject: [PATCH 09/10] docs(ci): note that a draft label needs a push --- AGENTS.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 8ac6974ef541..ae478cf9c1d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,6 +76,21 @@ 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 + +Draft PRs run a narrowed test matrix. +The `run-ci-backend` and `run-ci-frontend` labels force the full one, but a label alone does not start a run. +It applies on the next push, or when the PR is marked ready for review. +So after labeling a draft, push to start the run. An empty commit is enough: + +```bash +git commit --allow-empty -m "chore(ci): run the full matrix" && git push +``` + +Do not add `labeled` or `unlabeled` back to a merge gate's `on.pull_request.types` to avoid that push. +GitHub cannot filter a label trigger by name, so every unrelated label re-runs the full matrices against a commit CI already covered. +Guarding it inside the workflow is worse, not better: skipping the gate job cascades to the `if: always()` aggregator, which counts skipped dependencies as success and posts a green required check with no tests behind it. + #### Stacked PRs Restacking force-pushes every branch, and each push triggers a full CI fan-out. From 4b4b53e96f116154d072fb203db4a422158f6a6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Negr=C3=B3n?= Date: Wed, 29 Jul 2026 10:47:02 -0400 Subject: [PATCH 10/10] docs(ci): tighten the draft-matrix note --- AGENTS.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3968a26464d8..613db42455f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,18 +78,17 @@ Pushes still trigger CI, which burns runner credits, so batch related commits an #### Forcing the full CI matrix on a draft -Draft PRs run a narrowed test matrix. -The `run-ci-backend` and `run-ci-frontend` labels force the full one, but a label alone does not start a run. -It applies on the next push, or when the PR is marked ready for review. -So after labeling a draft, push to start the run. An empty commit is enough: +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. +An empty commit is enough. ```bash git commit --allow-empty -m "chore(ci): run the full matrix" && git push ``` -Do not add `labeled` or `unlabeled` back to a merge gate's `on.pull_request.types` to avoid that push. -GitHub cannot filter a label trigger by name, so every unrelated label re-runs the full matrices against a commit CI already covered. -Guarding it inside the workflow is worse, not better: skipping the gate job cascades to the `if: always()` aggregator, which counts skipped dependencies as success and posts a green required check with no tests behind it. +Do not add `labeled`/`unlabeled` back to a merge gate's `on.pull_request.types` to avoid that push. +GitHub cannot filter a label trigger by name, so every unrelated label re-runs the full matrices against a commit CI has already covered. +Guarding it inside the workflow is worse: skipping the gate job cascades to the `if: always()` aggregator, which counts a skipped dependency as success and posts a green required check with no tests behind it. #### Stacked PRs