From eb188897fee855fd15eb8e41df1fe7a32ec3ec71 Mon Sep 17 00:00:00 2001 From: Saqib Date: Fri, 14 Aug 2026 21:24:51 +0530 Subject: [PATCH 1/7] Add path-change detection job for CI (unwired) Adds a `changes` job (dorny/paths-filter) that outputs a boolean per test category (api/accessibility/visual/performance/load/e2e), plus a workflow_dispatch run_all override to bypass it for a manual full run. Not yet consumed by any suite job. --- .github/workflows/ci.yml | 169 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d59612..82f715d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,12 @@ on: pull_request: branches: [main] workflow_dispatch: + inputs: + run_all: + description: "Run every suite, bypassing path-based filtering" + type: boolean + required: false + default: false # One run at a time per PR (or per branch/ref for push/workflow_dispatch). # Added 2026-08-12: rapid pushes to an open PR were each auto-triggering a @@ -65,6 +71,169 @@ env: SANDBOX_ORG_SLUG: ${{ vars.SANDBOX_ORG_SLUG }} jobs: + # ────────────────────────────────────────────── Change detection + # Added 2026-08-14 so a PR/push only runs the suites whose test code (or + # its actual dependencies) changed — a docs-only change now runs lint and + # nothing else, instead of the full ~25min chain every time. + # + # Every gated job below reads this job's outputs through its own `if:`. + # Two failure modes both have to make downstream jobs run, not skip: + # 1. This job is deliberately skipped (workflow_dispatch run_all=true) — + # handled by the `if:` below, which skips this job entirely so + # `needs.changes.result` becomes 'skipped'. + # 2. This action errors outright (e.g. dorny/paths-filter itself breaks) + # — `needs.changes.result` becomes 'failure'. + # Every gated job's condition is `needs.changes.result != 'success' || + # needs.changes.outputs. == 'true'` — in both failure modes above + # the left side is true, so every suite runs (fail OPEN, not closed). A + # broken filter should never silently make every future PR pass CI having + # run zero tests. + changes: + name: Detect changed paths + runs-on: ubuntu-latest + if: github.event.inputs.run_all != 'true' + outputs: + api: ${{ steps.filter.outputs.api }} + accessibility: ${{ steps.filter.outputs.accessibility }} + visual: ${{ steps.filter.outputs.visual }} + performance: ${{ steps.filter.outputs.performance }} + load: ${{ steps.filter.outputs.load }} + e2e: ${{ steps.filter.outputs.e2e }} + steps: + - uses: actions/checkout@v6 + + - name: Filter changed paths by test category + id: filter + uses: dorny/paths-filter@v3 + with: + # workflow_dispatch has no natural "changed since what" — diff + # against main so a manual run without run_all still filters + # sensibly instead of erroring. + base: ${{ github.event_name == 'workflow_dispatch' && 'main' || '' }} + # Each category repeats a shared "universal" block of files whose + # fixtures/imports every suite's test collection depends on + # (verified via grep, not guessed) rather than using YAML anchors + # to de-duplicate it — this is the first path-filtering this repo + # has had; a second layer of YAML-anchor indirection on top of an + # already-new pattern isn't worth the maintenance cost it adds. + filters: | + api: + - 'tests/api/**' + - 'tests/data/**' + - 'conftest.py' + - 'tests/conftest.py' + - 'utils/config.py' + - 'utils/helpers.py' + - 'utils/report_generator.py' + - 'pages/base_page.py' + - 'pages/home_page.py' + - 'pages/login_page.py' + - 'locators/home_locators.py' + - 'locators/login_locators.py' + - 'pytest.ini' + - 'pyproject.toml' + - 'requirements.txt' + accessibility: + - 'tests/accessibility/**' + - 'utils/reporters.py' + - 'conftest.py' + - 'tests/conftest.py' + - 'utils/config.py' + - 'utils/helpers.py' + - 'utils/report_generator.py' + - 'pages/base_page.py' + - 'pages/home_page.py' + - 'pages/login_page.py' + - 'locators/home_locators.py' + - 'locators/login_locators.py' + - 'pytest.ini' + - 'pyproject.toml' + - 'requirements.txt' + visual: + - 'tests/visual/**' + - 'pages/evaluator_role_page.py' + - 'pages/new_evaluation_page.py' + - 'locators/evaluations_locators.py' + - 'locators/evaluator_role_locators.py' + - 'utils/visual_guards.py' + - 'conftest.py' + - 'tests/conftest.py' + - 'utils/config.py' + - 'utils/helpers.py' + - 'utils/report_generator.py' + - 'pages/base_page.py' + - 'pages/home_page.py' + - 'pages/login_page.py' + - 'locators/home_locators.py' + - 'locators/login_locators.py' + - 'pytest.ini' + - 'pyproject.toml' + - 'requirements.txt' + performance: + - 'tests/performance/**' + - 'pages/dashboard_page.py' + - 'pages/new_evaluation_page.py' + - 'utils/reporters.py' + - 'conftest.py' + - 'tests/conftest.py' + - 'utils/config.py' + - 'utils/helpers.py' + - 'utils/report_generator.py' + - 'pages/base_page.py' + - 'pages/home_page.py' + - 'pages/login_page.py' + - 'locators/home_locators.py' + - 'locators/login_locators.py' + - 'pytest.ini' + - 'pyproject.toml' + - 'requirements.txt' + load: + - 'tests/load/**' + - 'tests/data/**' + - 'pages/evaluations_page.py' + - 'pages/new_evaluation_page.py' + - 'locators/evaluations_locators.py' + - 'utils/reporters.py' + - 'conftest.py' + - 'tests/conftest.py' + - 'utils/config.py' + - 'utils/helpers.py' + - 'utils/report_generator.py' + - 'pages/base_page.py' + - 'pages/home_page.py' + - 'pages/login_page.py' + - 'locators/home_locators.py' + - 'locators/login_locators.py' + - 'pytest.ini' + - 'pyproject.toml' + - 'requirements.txt' + e2e: + - 'tests/e2e/**' + - 'tests/data/**' + - 'pages/**' + - 'locators/add_model_flow_locators.py' + - 'locators/dashboard_locators.py' + - 'locators/evaluation_detail_locators.py' + - 'locators/evaluations_locators.py' + - 'locators/evaluator_role_locators.py' + - 'locators/evaluators_locators.py' + - 'locators/models_locators.py' + - 'locators/prompt_libraries_locators.py' + - 'utils/test_data_factory.py' + - 'conftest.py' + - 'tests/conftest.py' + - 'utils/config.py' + - 'utils/helpers.py' + - 'utils/report_generator.py' + - 'pages/base_page.py' + - 'pages/home_page.py' + - 'pages/login_page.py' + - 'locators/home_locators.py' + - 'locators/login_locators.py' + - 'pytest.ini' + - 'pyproject.toml' + - 'requirements.txt' + # ────────────────────────────────────────────── Lint lint: name: Lint (ruff) From 75ffb00b95c4913899ef5b24ce25d69e69a35d4d Mon Sep 17 00:00:00 2001 From: Saqib Date: Fri, 14 Aug 2026 21:25:06 +0530 Subject: [PATCH 2/7] Gate api-tests on the changes job's api output First real consumer of the changes job: api-tests now skips when nothing under tests/api/ or its dependencies changed, with a fail-open guard against changes-job failure and an explicit lint success check (a skipped predecessor is otherwise ambiguous between "filtered out" and "lint failed upstream"). --- .github/workflows/ci.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82f715d..334ec90 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -280,7 +280,17 @@ jobs: api-tests: name: API Tests runs-on: ubuntu-latest - needs: lint + needs: [lint, changes] + # Path-filtered 2026-08-14 — see the `changes` job's header comment for + # the fail-open contract. `always() && !cancelled()` (not bare always()) + # is required so this job is still *evaluated* when path-filtered suites + # ahead of it in the chain get skipped, while still respecting a real + # concurrency-group cancellation — the same proven-safe pairing already + # used on test-summary; bare always() was proven unsafe here 2026-08-12. + if: | + always() && !cancelled() && + needs.lint.result == 'success' && + (needs.changes.result != 'success' || needs.changes.outputs.api == 'true') steps: - uses: actions/checkout@v6 From 32cd8734085e156b62c4f7df006dea5d219ead3d Mon Sep 17 00:00:00 2001 From: Saqib Date: Fri, 14 Aug 2026 21:25:28 +0530 Subject: [PATCH 3/7] Gate accessibility-tests and visual-tests on changed paths Extends the api-tests pattern down the existing chain. Reintroduces an explicit if: on these two jobs (they had none since 2026-08-12) using the proven-safe always()+!cancelled() pairing, not the bare always() that was removed back then. --- .github/workflows/ci.yml | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 334ec90..a4d8ae4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -396,15 +396,20 @@ jobs: accessibility-tests: name: Accessibility Tests (axe) runs-on: ubuntu-latest - needs: api-tests - # No if: always() here on purpose (removed 2026-08-12) - it was letting - # this job start even after the run was cancelled (verified live: a - # superseded run's api-tests correctly cancelled, but this job ignored - # that and kept running against the shared backend for up to its - # timeout). Every test step already uses continue-on-error, so - # api-tests' job conclusion is "success" even when its tests fail - - # default gating only actually skips this job on a genuine infra - # failure or cancellation, both cases where skipping is correct. + needs: [lint, api-tests, changes] + # Path-filtered 2026-08-14 — brings back an explicit `if:` (removed + # 2026-08-12 when bare always() was found to ignore cancellation; see + # api-tests' comment above for why always()+!cancelled() is safe where + # bare always() wasn't). needs.lint.result is checked directly (not just + # inferred via api-tests) because a skipped api-tests is ambiguous by + # itself — it means EITHER "path filter didn't match" (fine) OR "lint + # failed upstream and this cascaded" (must NOT run). Only a direct, + # unfiltered check against lint disambiguates that. + if: | + always() && !cancelled() && + needs.lint.result == 'success' && + (needs.api-tests.result == 'success' || needs.api-tests.result == 'skipped') && + (needs.changes.result != 'success' || needs.changes.outputs.accessibility == 'true') timeout-minutes: 60 steps: - uses: actions/checkout@v6 @@ -448,9 +453,13 @@ jobs: visual-tests: name: Visual Regression Tests runs-on: ubuntu-latest - needs: accessibility-tests - # No if: always() here either - see the comment on accessibility-tests' - # if: line above; same reasoning applies. + needs: [lint, accessibility-tests, changes] + # Path-filtered 2026-08-14 — same pattern as accessibility-tests above. + if: | + always() && !cancelled() && + needs.lint.result == 'success' && + (needs.accessibility-tests.result == 'success' || needs.accessibility-tests.result == 'skipped') && + (needs.changes.result != 'success' || needs.changes.outputs.visual == 'true') timeout-minutes: 60 steps: - uses: actions/checkout@v6 From de3ede2f168b87fbc0ba11d61650040df438bd42 Mon Sep 17 00:00:00 2001 From: Saqib Date: Fri, 14 Aug 2026 21:25:52 +0530 Subject: [PATCH 4/7] Add performance-tests and load-tests as new PR-gated CI jobs Neither ran on PRs before (performance was nightly-only; load ran nowhere). Inserted into the existing serial chain after visual-tests, with load-tests last-active since it's the suite most likely to stress the shared dev backend (concurrent-mutation/degradation tests per its own pytest.ini marker description). security and data were deliberately not given their own jobs: tests/api/test_security.py already carries pytest.mark.api (so api-tests' -m api already runs it), and tests/data/test_data.py has zero test functions (shared constants only). --- .github/workflows/ci.yml | 110 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4d8ae4..be7c53c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -258,9 +258,10 @@ jobs: # acquisition (scripts/_api_client.get_access_token) and a few # cookie-security tests drive a real headless Chromium via Playwright. # - # api-tests / accessibility-tests / visual-tests / e2e-tests run in a - # serial chain (via `needs:`) rather than all firing at once after lint. - # They all hit the same shared dev backend (dev.parakh.civicdataspace.in), + # api-tests / accessibility-tests / visual-tests / performance-tests / + # load-tests / e2e-tests run in a serial chain (via `needs:`) rather than + # all firing at once after lint. They all hit the same shared dev backend + # (dev.parakh.civicdataspace.in), # which has known slow-query issues (docs/app_bugs.md #13/#14) — running # them concurrently was overwhelming it and producing widespread transient # ReadTimeouts / stuck "Loading..." states that looked like test failures @@ -525,6 +526,109 @@ jobs: snapshots/ retention-days: 30 + # ────────────────────────────────────────────── Performance tests + # New 2026-08-14 — previously only ran nightly (scheduled.yml, run_perf + # toggle), never on PRs. Path-filtered like the suites above it in the + # chain; mirrors api-tests' job structure. + performance-tests: + name: Performance Tests + runs-on: ubuntu-latest + needs: [lint, visual-tests, changes] + if: | + always() && !cancelled() && + needs.lint.result == 'success' && + (needs.visual-tests.result == 'success' || needs.visual-tests.result == 'skipped') && + (needs.changes.result != 'success' || needs.changes.outputs.performance == 'true') + timeout-minutes: 60 + steps: + - uses: actions/checkout@v6 + + - name: Set up Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@v6 + with: + python-version: ${{ env.PYTHON_VERSION }} + cache: pip + + - name: Install Python dependencies + run: pip install -r requirements.txt + + - name: Install Playwright browsers + run: playwright install --with-deps chromium + + - name: Run performance tests + continue-on-error: true + run: | + pytest tests/performance/ \ + -v \ + --tb=short \ + --html=reports/performance_report.html \ + --self-contained-html \ + --json-report-file=reports/performance.json \ + -m performance + + - name: Upload performance test report + if: always() + uses: actions/upload-artifact@v7 + with: + name: performance-test-report + path: | + reports/performance_report.html + reports/performance.json + retention-days: 30 + + # ────────────────────────────────────────────── Load tests + # New 2026-08-14 — didn't run in any workflow before this (not even + # nightly). tests/load/** does concurrent-mutation/degradation testing + # against the shared dev backend (pytest.ini's own marker description), so + # it's placed last-active in the chain deliberately — nothing else in the + # same run executes after it, minimizing exposure if it does degrade the + # backend for the run's own later steps (there are none). + load-tests: + name: Load Tests + runs-on: ubuntu-latest + needs: [lint, performance-tests, changes] + if: | + always() && !cancelled() && + needs.lint.result == 'success' && + (needs.performance-tests.result == 'success' || needs.performance-tests.result == 'skipped') && + (needs.changes.result != 'success' || needs.changes.outputs.load == 'true') + timeout-minutes: 60 + steps: + - uses: actions/checkout@v6 + + - name: Set up Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@v6 + with: + python-version: ${{ env.PYTHON_VERSION }} + cache: pip + + - name: Install Python dependencies + run: pip install -r requirements.txt + + - name: Install Playwright browsers + run: playwright install --with-deps chromium + + - name: Run load tests + continue-on-error: true + run: | + pytest tests/load/ \ + -v \ + --tb=short \ + --html=reports/load_report.html \ + --self-contained-html \ + --json-report-file=reports/load.json \ + -m load + + - name: Upload load test report + if: always() + uses: actions/upload-artifact@v7 + with: + name: load-test-report + path: | + reports/load_report.html + reports/load.json + retention-days: 30 + # ────────────────────────────────────────────── Summary test-summary: name: Test Summary From f8836fe5dbbfc7b40b66a3c54669375df1d979e1 Mon Sep 17 00:00:00 2001 From: Saqib Date: Fri, 14 Aug 2026 21:26:08 +0530 Subject: [PATCH 5/7] Wire e2e-tests into the path-filtered chain, stays paused Extends needs: to include lint/load-tests/changes and adds the real gated if: condition as a ready-to-uncomment comment for whenever e2e is resumed. The active if: false pause (since 2026-08-11) is untouched. --- .github/workflows/ci.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be7c53c..42c5481 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -336,15 +336,25 @@ jobs: e2e-tests: name: E2E Tests (shard ${{ matrix.shard }}/${{ strategy.job-total }}) runs-on: ubuntu-latest - needs: visual-tests + needs: [lint, load-tests, changes] # PAUSED 2026-08-11 — remove this line to resume. test-summary's # E2E-report steps are guarded on `needs.e2e-tests.result != 'skipped'` # and degrade cleanly while this is off. Do NOT restore `if: always()` # (that was the original value) — removed the same pattern from # accessibility-tests/visual-tests 2026-08-12 after verifying live that - # it ignores workflow cancellation; default success()-gating is correct - # here for the same reason. + # it ignores workflow cancellation. + # + # Path filtering was added 2026-08-14 (see the `changes` job) to every + # other suite in this chain. When resuming e2e, delete the `if: false` + # line above and uncomment the real gated condition below instead of + # restoring plain always() — same always()+!cancelled() pairing used + # everywhere else in this file. if: false + # if: | + # always() && !cancelled() && + # needs.lint.result == 'success' && + # (needs.load-tests.result == 'success' || needs.load-tests.result == 'skipped') && + # (needs.changes.result != 'success' || needs.changes.outputs.e2e == 'true') timeout-minutes: 60 strategy: fail-fast: false From 2a75e891403932508899e7f3796ae5b6a67e43f9 Mon Sep 17 00:00:00 2001 From: Saqib Date: Fri, 14 Aug 2026 21:26:34 +0530 Subject: [PATCH 6/7] Include performance-tests and load-tests in the CI summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-summary now waits on both new jobs and reports their real test counts (via scripts/suite_summary.py, unchanged — it already handles a missing report gracefully) plus their raw job status in the collapsed details table. --- .github/workflows/ci.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42c5481..a51c4e0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -643,7 +643,7 @@ jobs: test-summary: name: Test Summary runs-on: ubuntu-latest - needs: [api-tests, e2e-tests, accessibility-tests, visual-tests] + needs: [api-tests, e2e-tests, accessibility-tests, visual-tests, performance-tests, load-tests] # Kept always() here (unlike the test jobs above) because this is the # reporting job, not a test job - it must still run while e2e-tests is # deliberately paused (if: false -> always "skipped", and default @@ -713,7 +713,9 @@ jobs: "API=suite-artifacts/api.json" \ "E2E=shard-artifacts/*/reports/e2e_shard_*.json" \ "Accessibility=suite-artifacts/accessibility.json" \ - "Visual Regression=suite-artifacts/reports/visual.json" + "Visual Regression=suite-artifacts/reports/visual.json" \ + "Performance=suite-artifacts/performance.json" \ + "Load=suite-artifacts/load.json" echo "" echo "**Platform:** ${{ env.BASE_URL }}" echo "" @@ -725,6 +727,8 @@ jobs: echo "| E2E Tests | ${{ needs.e2e-tests.result }} |" echo "| Accessibility | ${{ needs.accessibility-tests.result }} |" echo "| Visual Regression | ${{ needs.visual-tests.result }} |" + echo "| Performance | ${{ needs.performance-tests.result }} |" + echo "| Load | ${{ needs.load-tests.result }} |" echo "" echo "" } >> "$GITHUB_STEP_SUMMARY" From ad285f0a1da724383d96916817cc4ab44c5f70bc Mon Sep 17 00:00:00 2001 From: Saqib Date: Fri, 14 Aug 2026 21:27:06 +0530 Subject: [PATCH 7/7] Document path-filtered CI in ci_workflow_notes.md Explains the changes job, the fail-open contract, the skip/failure- ambiguity gotcha that motivates checking lint's result directly in every gated job, and why always()+!cancelled() reappearing on accessibility-tests/visual-tests isn't a regression of the 2026-08-12 bare-always() bug. --- docs/ci_workflow_notes.md | 88 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 3 deletions(-) diff --git a/docs/ci_workflow_notes.md b/docs/ci_workflow_notes.md index 7a4e677..34ce27b 100644 --- a/docs/ci_workflow_notes.md +++ b/docs/ci_workflow_notes.md @@ -71,10 +71,12 @@ code or contort a naming scheme. | Job | Status | Notes | |---|---|---| -| `e2e-tests` | **paused** since 2026-08-11 (`if: false`) | By request, not due to failures. `test-summary`'s E2E-report steps are guarded on `needs.e2e-tests.result != 'skipped'` so the pipeline degrades cleanly on missing shard artifacts. To resume, delete the `if: false` line to restore `if: always()`; the guards are harmless and can stay. | +| `e2e-tests` | **paused** since 2026-08-11 (`if: false`) | By request, not due to failures. `test-summary`'s E2E-report steps are guarded on `needs.e2e-tests.result != 'skipped'` so the pipeline degrades cleanly on missing shard artifacts. To resume: delete the active `if: false` line and uncomment the real gated condition already sitting commented-out directly below it in `ci.yml` — do **not** restore `if: always()`, see the "Path-filtered CI" section below for why. | -Note the chain `e2e-tests needs: visual-tests` — resuming `e2e-tests` alone works -regardless of `visual-tests`, since the job's own `if:` controls it. +`e2e-tests` now depends on `needs: [lint, load-tests, changes]` (as of the +2026-08-14 path-filtering change below) rather than just `visual-tests` — +resuming it still works independent of any of those, since the job's own +`if:` is what actually controls it. --- @@ -134,3 +136,83 @@ recurrence of that exact pattern rather than treating a generic "it failed" as sufficient — and should isolate whether it's `api-tests`' fixture load specifically, since `accessibility-tests`/`visual-tests` showed no such symptom running concurrently with it in this same run. + +--- + +## Path-filtered CI (2026-08-14) + +**Status: implemented, first path-filtering this repo has ever had.** + +`ci.yml` previously ran the full chain — `api-tests` → `accessibility-tests` +→ `visual-tests` → `e2e-tests` — on every push/PR regardless of what changed. +A new `changes` job (`dorny/paths-filter@v3`) now detects which of six +categories (`api`, `accessibility`, `visual`, `performance`, `load`, `e2e`) +actually changed, and every suite job is gated on its own category. Two new +suites were added at the same time: `performance-tests` and `load-tests` +(previously performance only ran nightly via `scheduled.yml`; load didn't +run in any workflow at all). + +Two categories from the original request were deliberately **not** turned +into their own jobs: + +- **security** is a pytest *marker*, not a directory — + `tests/api/test_security.py` carries `pytestmark = [pytest.mark.api, + pytest.mark.security]`, so `api-tests`' existing `-m api` run already + executes every security test. A dedicated job would just re-run a subset + of `api-tests`. +- **data** (`tests/data/test_data.py`) has zero `def test_` functions — it's + a shared `TestGraphQL`/`TestUsers`/etc. constants class. A `data-tests` job + would collect 0 tests every run. `tests/data/**` changes instead route + into the `api`, `e2e`, and `load` filters (the suites whose test files + actually import `TestGraphQL`). + +### Fail-open, not fail-closed + +If the `changes` job errors, or is deliberately skipped via the +`workflow_dispatch` `run_all` input, every downstream job must still run — +a broken filter should never silently make every future PR pass CI having +run zero tests. Every gated job's condition includes: + +```yaml +(needs.changes.result != 'success' || needs.changes.outputs. == 'true') +``` + +`changes.result` is `'skipped'` under `run_all` (its own `if:` skips it) and +would be `'failure'` if the action itself errors — both `!= 'success'`, so +every category's clause is satisfied and every suite runs. + +### The skip/failure-ambiguity gotcha (the reason `lint` is checked directly everywhere) + +`needs..result == 'skipped'` is true for two completely different +reasons that are otherwise indistinguishable: (1) that job's own path filter +just didn't match — expected, fine — or (2) an *upstream* job genuinely +failed, e.g. `lint` breaks, which makes every job after it (whose `if:` now +includes `always()`) evaluate to `'skipped'` too, cascading the same value +down the whole chain. Checking only "did my immediate predecessor +succeed-or-skip" can't tell these apart. + +The fix used throughout `ci.yml`: every gated job adds `lint` to its own +`needs:` and checks `needs.lint.result == 'success'` **directly**, not just +inferred transitively through its predecessor. `lint` is never path-filtered, +so its result is always unambiguous. This is why every gated job's `needs:` +list has three entries (`lint`, its immediate predecessor, `changes`) even +though `lint` isn't otherwise in that job's critical path — it's there +purely to make the direct reference legal. + +**Verify this specifically, don't just trust the design**: intentionally +break `ruff` (e.g. commit an unused import) together with an unrelated +`tests/api/**` change, push, and confirm every downstream job shows +`skipped` — not that they ran anyway. If any of them run, the fail-open +logic has a real bug, not just a theoretical one. + +### Why `always() && !cancelled()` reappears on accessibility-tests/visual-tests + +These two jobs had **no** `if:` at all (relying on default `success()` +gating) since the 2026-08-12 fix documented above, specifically to avoid the +bare-`always()`-ignores-cancellation bug. Path filtering requires an +explicit `if:` again (a job needs custom logic to still be *evaluated* when +its predecessor was filtered-out-skipped rather than genuinely successful). +This is safe because it uses the same `always() && !cancelled()` pairing +already proven safe on `test-summary` — never bare `always()`. If you see +bare `always()` reappear on any suite job in this file, that is the +2026-08-12 bug coming back, not this change.