ci: parallelise the test suite and stop paying for superseded runs

.github/workflows/ci.yml

@@ -6,6 +6,33 @@ on:
   pull_request:
     branches: [main]
 
+# One in-flight run per PR. Pushing again to an open PR supersedes the previous
+# run, which used to keep burning a full ~39 billed minutes (a Windows job bills
+# 2x) on a commit nobody would ever look at again. Measured over 13 days: 29 of
+# 219 PR runs (13%) were still executing when the next push landed, and a single
+# iterating branch racked up 23 runs.
+#
+# A push to main is the post-merge verification of a commit that is already
+# shipped, so it must never be dropped: losing the signal for the commit in
+# between two merges is exactly when you want it.
+#
+# `cancel-in-progress: false` is NOT enough to guarantee that. It protects a
+# RUNNING run; GitHub keeps at most one running plus one PENDING run per group
+# and, quoting the workflow-syntax docs, "any existing `pending` job or workflow
+# in the same concurrency group will be canceled and the new queued job or
+# workflow will take its place". So with every main push sharing one
+# ref-keyed group, three merges landing inside one run duration silently cancel
+# the middle commit's queued run. Not hypothetical here: 6 of the last 120 main
+# pushes sat in a 3-merges-within-5-minutes window, one burst landing five
+# merges ~15s apart.
+#
+# Keying pushes on the SHA gives every main commit its own group, so it can
+# never be the pending run that gets displaced. PRs stay keyed on the ref, which
+# is what makes a new push supersede the previous run.
+concurrency:
+  group: ci-${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
 jobs:
   # ────────────────────────────────────────────────────────────────────────
   # Static analysis + silent-drift gates. These are deterministic and
@@ -31,6 +58,8 @@ jobs:
           # github.com/ghraw on every run -- that fetch is rate-limited
           # and flaky on shared CI egress IPs (it killed the Windows job once).
           version: "0.11.16"
+          enable-cache: true
+          cache-dependency-glob: "uv.lock"
 
       - uses: actions/setup-python@v6
         with:
@@ -171,22 +200,35 @@ jobs:
         run: uv run python scripts/check_file_size.py
 
   # ────────────────────────────────────────────────────────────────────────
-  # Test suite across every supported interpreter. pyproject declares
-  # `requires-python = ">=3.12"`, so the matrix is 3.12 + 3.13 (3.10/3.11 are
-  # intentionally out of scope). `integration` tests are deselected; `e2e`
-  # tests self-skip without credentials (the dedicated e2e.yml workflow runs
-  # them nightly against a real project). Coverage is INFORMATIONAL: the
-  # term-missing report is printed for visibility but NO --cov-fail-under
-  # threshold is enforced, so it never blocks a merge.
+  # Test suite. pyproject declares `requires-python = ">=3.12"`, so the
+  # supported interpreters are 3.12 + 3.13 (3.10/3.11 are intentionally out of
+  # scope). `integration` tests are deselected; `e2e` tests self-skip without
+  # credentials (the dedicated e2e.yml workflow runs them nightly against a
+  # real project).
+  #
+  # WHICH interpreters run depends on the event:
+  #   * pull_request -> 3.12 only (the floor, and what the wheel is built on)
+  #   * push to main -> 3.12 + 3.13
+  # A 3.13-only regression is therefore caught on main rather than on the PR.
+  # That is an accepted trade: the two interpreters differ in nothing this
+  # codebase touches, a PR-time 3.13 job cost ~8 billed minutes of the ~39 a PR
+  # used to spend, and main runs unattended anyway. If 3.13 ever starts
+  # diverging in practice, put it back in the PR matrix -- that is a one-word
+  # change to the expression below.
+  #
+  # Coverage is INFORMATIONAL -- no --cov-fail-under threshold is enforced, so
+  # it can never block a merge. It also costs ~50% extra wall clock (measured:
+  # 152s -> 230s sequentially), so it is collected on main pushes only. Locally
+  # it stays one `make test-cov` away.
   # ────────────────────────────────────────────────────────────────────────
   test:
     runs-on: ubuntu-latest
     strategy:
       # Don't cancel 3.12 just because 3.13 tripped (or vice versa) -- we want
-      # to see which interpreters pass on a given PR, not just the first failure.
+      # to see which interpreters pass on a given run, not just the first failure.
       fail-fast: false
       matrix:
-        python-version: ["3.12", "3.13"]
+        python-version: ${{ github.event_name == 'pull_request' && fromJSON('["3.12"]') || fromJSON('["3.12", "3.13"]') }}
     env:
       # Pin uv's interpreter to the matrix version for BOTH `sync` and `run`,
       # so the suite is actually exercised on each interpreter instead of
@@ -198,6 +240,8 @@ jobs:
       - uses: astral-sh/setup-uv@v7
         with:
           version: "0.11.16"
+          enable-cache: true
+          cache-dependency-glob: "uv.lock"
 
       - uses: actions/setup-python@v6
         with:
@@ -206,8 +250,23 @@ jobs:
       - name: Install dependencies
         run: uv sync --extra server
 
+      # `-n auto` fans the ~6k tests across the runner's cores (4 on a standard
+      # GitHub-hosted runner). Every HTTP call in this suite is mocked and the
+      # tests hold no shared mutable state, so it parallelises cleanly --
+      # measured locally at 152s -> 37s on 4 workers, byte-identical results
+      # over repeated runs. `-v` is dropped: under xdist it interleaves 6k
+      # lines from four workers into unreadable output, and a failure prints
+      # its own node id anyway.
+      - name: Tests
+        if: github.event_name != 'push'
+        run: uv run pytest tests/ -m "not integration" -n auto
+
+      # Same run, plus the informational coverage report. pytest-cov combines
+      # the per-worker data files automatically, so the totals match a
+      # sequential run exactly (verified: 31296/5405/9186/1121, 80%).
       - name: Tests (with coverage report)
-        run: uv run pytest tests/ -v -m "not integration" --cov --cov-report=term-missing
+        if: github.event_name == 'push'
+        run: uv run pytest tests/ -m "not integration" -n auto --cov --cov-report=term-missing
 
   # Real wheel build and focused export regression on Windows -- the only
   # place the issue #320 and #529 fixes can be verified against real Windows
@@ -228,6 +287,8 @@ jobs:
           # github.com/ghraw on every run -- that fetch is rate-limited
           # and flaky on shared CI egress IPs (it killed the Windows job once).
           version: "0.11.16"
+          enable-cache: true
+          cache-dependency-glob: "uv.lock"
 
       - uses: actions/setup-python@v6
         with:
@@ -247,29 +308,6 @@ jobs:
       - name: Build wheel WITH bundled UI
         run: uv build --wheel
 
-      # Windows does not expose os.O_NOFOLLOW. Exercise the focused export
-      # regression here, where the portable flags run on the real platform.
-      - name: Test semantic-layer export on Windows (issue #529)
-        run: uv run pytest tests/test_semantic_layer_service.py -k "export" -v
-
-      # The self-update helper is a PowerShell script authored on machines that
-      # cannot execute it. This suite runs it for real: it proves the script
-      # parses, that it records the installer's exit code, and -- the branch
-      # that actually protects the environment -- that it installs NOTHING
-      # while a watched process is still alive (issue #528). The rest of the
-      # suite also runs here with `should_defer()` returning its real Windows
-      # default and a real detached spawn.
-      - name: Test the deferred self-update helper on Windows (issue #528)
-        run: uv run pytest tests/test_update_runner.py -v
-
-      # `os.replace()` cannot rename over a file Windows still holds open, so a
-      # store that locks its own state file dies with WinError 5 on every write
-      # while passing everywhere on POSIX. That shipped: `job run
-      # --idempotency-key` was unusable on Windows for a full release. Anything
-      # doing lock-then-atomic-replace has to prove it here.
-      - name: Test lock-then-replace file stores on Windows (issue #427)
-        run: uv run pytest tests/test_job_idempotency_store.py -v
-
       # The whole suite, as a gate. It never ran here before: a real run was 55
       # failures, and nobody can gate on that, so nothing did -- which let real
       # Windows defects sit in the noise for entire releases. Two of them did:
@@ -281,20 +319,57 @@ jobs:
       # fcntl does not exist there). Keeping this green is the point: a red
       # suite nobody can act on is worse than no suite, because it reads as
       # coverage while hiding things.
-      # The targeted steps above run against the default dependency set, but
-      # the full suite imports the `serve` tests, so it needs the same extras
+      # The specific regressions this job exists to hold down are all inside the
+      # suite below, and used to ALSO run as three separate named steps before
+      # it (`-k export` for #529, test_update_runner.py for #528,
+      # test_job_idempotency_store.py for #427). Those were strict subsets of
+      # this run -- duplicated Windows minutes, billed at 2x -- so they are
+      # gone. What they covered has not changed:
+      #   * #529: Windows has no os.O_NOFOLLOW, so semantic-layer export must
+      #           prove its portable flags on the real platform.
+      #   * #528: the deferred self-update helper is a PowerShell script
+      #           authored on machines that cannot run it; here it really
+      #           executes, with a real detached spawn, and must install
+      #           NOTHING while a watched process is still alive.
+      #   * #427: os.replace() cannot rename over a file Windows still holds
+      #           open, so lock-then-atomic-replace stores die with WinError 5
+      #           on every write while passing everywhere on POSIX. That
+      #           shipped once -- `job run --idempotency-key` was unusable on
+      #           Windows for a full release.
+      # A failure now names the test node id instead of the step, which is the
+      # same information.
+      #
+      # The full suite imports the `serve` tests, so it needs the same extras
       # the Linux job installs. Without this the gate fails on
       # `ModuleNotFoundError: No module named 'fastapi'` -- an artefact of the
       # job's own setup rather than anything about Windows.
       - name: Install server extras for the full run
         run: uv sync --extra server
 
+      # `-n auto`, same as the Linux jobs: this step was 477s of the job's 568s,
+      # and Windows minutes bill at 2x, so it was the single most expensive
+      # thing in the whole PR pipeline.
+      #
+      # Read a Windows-only flake here carefully before blaming xdist. The
+      # first run of this change surfaced exactly one failure, and it was a
+      # test asserting a 250ms wall-clock margin (test_auth_pkce), not a
+      # parallelism-safety bug -- four busy workers just made the stall that
+      # the margin never tolerated actually happen. That test was widened;
+      # check for the same shape first.
+      #
+      # If something genuinely does turn out to be parallel-unsafe, drop back
+      # to sequential by replacing `-n auto` with `-p no:xdist` -- do NOT
+      # disable the suite. Windows-only file-locking and atomic-replace semantics are
+      # exactly what it is here to catch, and parallel workers touching the
+      # same tmp paths are the plausible failure mode. The Linux jobs are
+      # unaffected either way.
       - name: Full test suite on Windows
         run: >
           uv run pytest tests/
           --ignore=tests/test_e2e.py
           --ignore=tests/test_e2e_auth.py
           --ignore=tests/test_server_semantic_layer_routes_e2e.py
+          -n auto
           -q
 
       - name: Assert the SPA is bundled (Bug 1 fixed)

.github/workflows/frontend.yml

@@ -35,6 +35,15 @@ on:
       - "web/**"
       - ".github/workflows/frontend.yml"
 
+# Same rationale, and the same pending-run caveat, as ci.yml: supersede an
+# in-flight PR run when the branch is pushed again, but never drop a main push.
+# Pushes are keyed on the SHA rather than the ref because `cancel-in-progress:
+# false` only protects a RUNNING run -- a PENDING one is cancelled when a newer
+# run queues into the same group. See the longer note in ci.yml.
+concurrency:
+  group: frontend-${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
 jobs:
   frontend:
     name: Type check + test + build (web/frontend)

Makefile

@@ -14,11 +14,16 @@ install-server: ## Install FastAPI/uvicorn for `kbagent serve` (web UI backend)
 sync: ## Sync dependencies from lockfile
 	uv sync
 
+# `-n auto` fans the suite across every core (~6k tests, all HTTP mocked, no
+# shared mutable state). Measured 152s -> 29s on an 11-core machine. `-v` is
+# dropped with it: interleaved per-worker output is unreadable, and a failing
+# test prints its own node id. Use `make test-file FILE=...` for a sequential,
+# verbose run while debugging a single file.
 test: ## Run all tests (excluding e2e — use test-e2e separately)
-	uv run pytest tests/ -v -m "not e2e"
+	uv run pytest tests/ -m "not e2e" -n auto
 
 test-unit: ## Run unit tests only (exclude integration and e2e)
-	uv run pytest tests/ -v -m "not integration and not e2e"
+	uv run pytest tests/ -m "not integration and not e2e" -n auto
 
 test-integration: ## Run integration tests only
 	uv run pytest tests/ -v -m integration
@@ -46,7 +51,7 @@ test-file: ## Run a specific test file (FILE=tests/test_cli.py)
 	uv run pytest $(FILE) -v
 
 test-cov: ## Run the unit suite with a coverage report (informational; no threshold gate)
-	uv run pytest tests/ -v -m "not integration" --cov --cov-report=term-missing
+	uv run pytest tests/ -m "not integration" -n auto --cov --cov-report=term-missing
 
 lint: ## Run ruff linter
 	uv run ruff check src/ tests/ scripts/

pyproject.toml

@@ -138,6 +138,12 @@ dev = [
     "pytest-cov>=5",
     "pytest-httpx>=0.30",
     "pytest-asyncio>=0.23",
+    # Parallel test execution. The suite is ~6k tests of in-process CliRunner
+    # invocations with every HTTP call mocked -- CPU-bound, no shared mutable
+    # state, so it scales almost linearly across workers. Sequentially it took
+    # ~8.5 min per interpreter in CI (three such runs per PR, one of them on a
+    # 2x-billed Windows runner) purely because nothing ever ran concurrently.
+    "pytest-xdist>=3.6",
     "ruff>=0.8",
     "ty>=0.0.33",
     "bandit>=1.9.4",

tests/test_auth_pkce.py

@@ -195,18 +195,36 @@ def test_timeout_raises_pkce_callback_timeout(self) -> None:
     def test_pkce_callback_timeout_is_fallback_eligible(self) -> None:
         assert issubclass(PkceCallbackTimeout, PkceSetupError)
 
-    def test_callback_arriving_just_before_timeout_succeeds(self) -> None:
+    def test_callback_arriving_before_timeout_succeeds(self) -> None:
+        """A callback that lands inside the deadline resolves the wait.
+
+        The margin between the callback (0.05s) and the deadline is deliberately
+        wide. It used to be 0.3s, which passed on an idle machine and failed on a
+        busy one: under parallel CI workers on Windows a 250ms scheduling stall
+        is ordinary, and the test then reported a callback-handling bug that did
+        not exist. A generous deadline costs nothing here -- wait() returns the
+        moment the callback arrives, not when the timeout expires -- and nothing
+        is lost by widening it, because that the deadline is HONOURED is what
+        test_timeout_raises_pkce_callback_timeout and the sibling below assert.
+        """
         with PkceCallbackServer(expected_state="expected-state") as server:
             _get_after(0.05, server.redirect_uri, {"code": "on-time", "state": "expected-state"})
-            result = server.wait(timeout=0.3)
+            result = server.wait(timeout=5.0)
 
         assert result.code == "on-time"
 
-    def test_callback_arriving_just_after_timeout_is_not_observed(self) -> None:
+    def test_callback_arriving_after_timeout_is_not_observed(self) -> None:
         """A callback scheduled to land after the (short, injected) timeout must
-        not be picked up -- wait() raises PkceCallbackTimeout on schedule."""
+        not be picked up -- wait() raises PkceCallbackTimeout on schedule.
+
+        Same widened margin, for the same reason, in the other direction: the
+        callback must be comfortably later than the deadline even when the box
+        stalls. This does NOT make the test slow -- wait() raises after 0.1s and
+        the block exits; the pending callback then fires from a daemon timer
+        against a closed server, which `_get` swallows by design.
+        """
         with PkceCallbackServer(expected_state="expected-state") as server:
-            _get_after(0.4, server.redirect_uri, {"code": "too-late", "state": "expected-state"})
+            _get_after(5.0, server.redirect_uri, {"code": "too-late", "state": "expected-state"})
 
             with pytest.raises(PkceCallbackTimeout):
                 server.wait(timeout=0.1)