From 5c3cce6808f22aab63ed873762449b6a8a1d82c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Thu, 28 May 2026 09:52:25 -0700 Subject: [PATCH 1/8] ci: dedupe quality gates and add fast-fail pre-check (AE-3161) Eliminates redundant CI/CD work that was costing ~24 min of compute per merge and ~28 min per release-please bot PR push. Workflow changes: - ci.yml: add concurrency group (cancel in-flight PR runs), paths-ignore for release-please bot PRs, new pre-check job (uvx ruff format+check, ~10s no-install fast-fail), bump setup-uv v2 -> v5, build job only on push:main since release-please.yml builds + publishes the same SHA. - release-please.yml: remove duplicate quality-gates job. Branch protection on main already enforces ci.yml green before code lands, so the second matrix validated nothing. Saves a full 4-leg matrix per merge. - e2e.yml: drop unit-tests job (duplicated ci.yml quality-gates), fold the summary step into the e2e job to remove the third workflow definition of the same install setup. Makefile changes: - dev: drop redundant 'uv pip install -e .' (uv sync handles editable install in workspace mode). - quality-check: alias ci-quality-github so there is one canonical CI quality gate, no drift between local and CI. - ci-quality-github / test-coverage: fix the latent junit overwrite bug where both pytest invocations wrote pytest-results.xml -- now writes pytest-results-parallel.xml and pytest-results-serial.xml. The double pytest pass is retained because it provides process isolation between parallel and serial tests; collapsing to a single loadgroup run surfaced state pollution in test_load_balancer_sls_stub. Note: removing quality-gates from release-please.yml + paths-ignore for the bot's files means the release-please PR will have no required CI check. Branch protection on main will need to allow missing checks for the release-please--* branch pattern, or add a no-op sentinel job named to match the protection rule. https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 68 ++++++--- .github/workflows/e2e.yml | 210 ++++++--------------------- .github/workflows/release-please.yml | 59 ++------ Makefile | 24 +-- 4 files changed, 119 insertions(+), 242 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e289aa1..d484a071 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,14 @@ name: CI on: pull_request: - branches: [ main ] + branches: [main] + # release-please bot PRs only touch CHANGELOG / version manifest -- skip + # the full matrix; release-please.yml will exercise publishing on merge. + paths-ignore: + - 'CHANGELOG.md' + - '.release-please-manifest.json' push: - branches: [ main ] + branches: [main] permissions: contents: read @@ -12,19 +17,43 @@ permissions: issues: write actions: read +# Cancel in-flight PR runs when a new commit is pushed; never cancel main runs. +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + env: PYTHON_VERSION: '3.11' jobs: + # Fast-fail formatting/lint check. ~10s, no project install. Catches the + # common case (ruff format/check fail) before paying for the full matrix. + pre-check: + name: Pre-check (format + lint) + runs-on: ubuntu-latest + timeout-minutes: 3 + if: ${{ !startsWith(github.head_ref, 'release-please--') }} + steps: + - uses: actions/checkout@v4 + - uses: astral-sh/setup-uv@v5 + with: + enable-cache: true + cache-dependency-glob: pyproject.toml + - name: Ruff format + run: uvx ruff format --check . + - name: Ruff lint + run: uvx ruff check . --output-format=github + quality-gates: name: Quality Gates runs-on: ubuntu-latest + needs: [pre-check] strategy: fail-fast: false matrix: python-version: ['3.10', '3.11', '3.12', '3.13'] timeout-minutes: 15 - + steps: - name: Checkout code uses: actions/checkout@v4 @@ -35,14 +64,14 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install uv - uses: astral-sh/setup-uv@v2 + uses: astral-sh/setup-uv@v5 with: enable-cache: true - cache-dependency-glob: "pyproject.toml" + cache-dependency-glob: pyproject.toml - name: Install dependencies run: make dev - + - name: Quality checks run: make ci-quality-github @@ -51,27 +80,27 @@ jobs: if: always() with: name: test-results-${{ matrix.python-version }} - path: pytest-results.xml + path: pytest-results-*.xml + # Validate the wheel actually builds. Only on push:main since release-please.yml + # builds + publishes from the same SHA; a PR-time wheel build would add ~1 min + # for no extra signal beyond quality-gates. build: name: Build Package runs-on: ubuntu-latest needs: [quality-gates] + if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} timeout-minutes: 5 - - steps: - - name: Checkout code - uses: actions/checkout@v4 - - name: Set up Python - uses: actions/setup-python@v5 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: python-version: ${{ env.PYTHON_VERSION }} - - - name: Install uv - uses: astral-sh/setup-uv@v2 + - uses: astral-sh/setup-uv@v5 with: enable-cache: true + cache-dependency-glob: pyproject.toml - name: Build package run: make build @@ -81,10 +110,3 @@ jobs: - name: Validate wheel packaging run: ./scripts/validate-wheel.sh - - - name: Upload build artifacts - uses: actions/upload-artifact@v4 - with: - name: dist - path: dist/ - diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index dda41a10..2c78be57 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -1,11 +1,6 @@ name: E2E Tests on: - # Uncomment to run on every push to main / pull request: - # push: - # branches: [main] - # pull_request: - # branches: [main] workflow_dispatch: inputs: tests: @@ -20,74 +15,25 @@ env: PYTHON_VERSION: '3.11' jobs: - unit-tests: - name: Unit + Integration - runs-on: ubuntu-latest - timeout-minutes: 15 - - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: ${{ env.PYTHON_VERSION }} - - - name: Install uv - uses: astral-sh/setup-uv@v5 - with: - enable-cache: true - cache-dependency-glob: "pyproject.toml" - - - name: Install dependencies - run: uv sync --all-groups - - - name: Run unit + integration tests - run: | - uv run pytest tests/ \ - -n auto \ - --timeout=60 \ - --junitxml=unit-results.xml \ - --cov-report=xml:coverage.xml \ - --cov-fail-under=0 - - - name: Upload test results - uses: actions/upload-artifact@v4 - if: always() - with: - name: unit-results - path: unit-results.xml - - - name: Upload coverage - uses: actions/upload-artifact@v4 - if: always() - with: - name: coverage - path: coverage.xml - e2e: name: E2E runs-on: ubuntu-latest timeout-minutes: 45 steps: - - name: Checkout code - uses: actions/checkout@v4 + - uses: actions/checkout@v4 - - name: Set up Python - uses: actions/setup-python@v5 + - uses: actions/setup-python@v5 with: python-version: ${{ env.PYTHON_VERSION }} - - name: Install uv - uses: astral-sh/setup-uv@v5 + - uses: astral-sh/setup-uv@v5 with: enable-cache: true - cache-dependency-glob: "pyproject.toml" + cache-dependency-glob: pyproject.toml - name: Install dependencies - run: uv sync --all-groups + run: make dev - name: Run E2E tests env: @@ -118,10 +64,10 @@ jobs: tests = int(root.attrib.get("tests", 0)) print(f"Tests run: {tests}") if tests == 0: - print("ERROR: 0 tests ran — check test filter or test collection") + print("ERROR: 0 tests ran -- check test filter or test collection") sys.exit(1) except FileNotFoundError: - print("ERROR: e2e-results.xml not found — pytest did not run") + print("ERROR: e2e-results.xml not found -- pytest did not run") sys.exit(1) EOF @@ -132,36 +78,8 @@ jobs: name: e2e-results path: e2e-results.xml - summary: - name: Summary - needs: [unit-tests, e2e] - if: always() - runs-on: ubuntu-latest - timeout-minutes: 5 - - steps: - - name: Download unit results - uses: actions/download-artifact@v4 - with: - name: unit-results - continue-on-error: true - - - name: Download coverage - uses: actions/download-artifact@v4 - with: - name: coverage - continue-on-error: true - - - name: Download E2E results - uses: actions/download-artifact@v4 - with: - name: e2e-results - continue-on-error: true - - name: Write summary - env: - UNIT_RESULT: ${{ needs.unit-tests.result }} - E2E_RESULT: ${{ needs.e2e.result }} + if: always() run: | python - <<'EOF' import xml.etree.ElementTree as ET, os, sys @@ -169,79 +87,47 @@ jobs: summary_file = os.environ.get("GITHUB_STEP_SUMMARY") out = open(summary_file, "a") if summary_file else sys.stdout - def parse_junit(path): - """Return (total, failures, duration) from a JUnit XML file.""" - try: - root = ET.parse(path).getroot() - suites = root.findall("testsuite") if root.tag == "testsuites" else [root] - total = sum(int(s.attrib.get("tests", 0)) for s in suites) - failures = sum(int(s.attrib.get("failures", 0)) + int(s.attrib.get("errors", 0)) for s in suites) - duration = sum(float(s.attrib.get("time", 0)) for s in suites) - failed_names = [ - tc.get("classname", "") + "::" + tc.get("name", "") - for s in suites - for tc in s.findall("testcase") - if tc.find("failure") is not None or tc.find("error") is not None - ] - return total, failures, duration, failed_names - except FileNotFoundError: - return None, None, None, [] - - def status_icon(result, total, failures): - if total is None: return ":x: Did not run" - if total == 0: return ":warning: No tests ran" - if failures == 0: return ":white_check_mark: Passed" - return ":x: Failed" - - unit_total, unit_fail, unit_dur, unit_failed_names = parse_junit("unit-results.xml") - e2e_total, e2e_fail, e2e_dur, e2e_failed_names = parse_junit("e2e-results.xml") - - unit_pass = (unit_total - unit_fail) if unit_total is not None else None - e2e_pass = (e2e_total - e2e_fail) if e2e_total is not None else None - - unit_result = os.environ.get("UNIT_RESULT", "") - e2e_result = os.environ.get("E2E_RESULT", "") - - print("# Test Results\n", file=out) - print("| Suite | Status | Passed | Failed | Total | Duration |", file=out) - print("|---|---|---|---|---|---|", file=out) - print(f"| Unit + Integration | {status_icon(unit_result, unit_total, unit_fail)} | " - f"{unit_pass if unit_pass is not None else '-'} | " - f"{unit_fail if unit_fail is not None else '-'} | " - f"{unit_total if unit_total is not None else '-'} | " - f"{unit_dur:.1f}s |" if unit_dur is not None else "- |", file=out) - print(f"| E2E | {status_icon(e2e_result, e2e_total, e2e_fail)} | " - f"{e2e_pass if e2e_pass is not None else '-'} | " - f"{e2e_fail if e2e_fail is not None else '-'} | " - f"{e2e_total if e2e_total is not None else '-'} | " - f"{e2e_dur:.1f}s |" if e2e_dur is not None else "- |", file=out) + try: + root = ET.parse("e2e-results.xml").getroot() + suites = root.findall("testsuite") if root.tag == "testsuites" else [root] + total = sum(int(s.attrib.get("tests", 0)) for s in suites) + failures = sum(int(s.attrib.get("failures", 0)) + int(s.attrib.get("errors", 0)) for s in suites) + duration = sum(float(s.attrib.get("time", 0)) for s in suites) + failed_names = [ + tc.get("classname", "") + "::" + tc.get("name", "") + for s in suites + for tc in s.findall("testcase") + if tc.find("failure") is not None or tc.find("error") is not None + ] + except FileNotFoundError: + total, failures, duration, failed_names = None, None, None, [] + + if total is None: + status = ":x: Did not run" + elif total == 0: + status = ":warning: No tests ran" + elif failures == 0: + status = ":white_check_mark: Passed" + else: + status = ":x: Failed" + + passed = (total - failures) if total is not None else None + + print("# E2E Results\n", file=out) + print("| Status | Passed | Failed | Total | Duration |", file=out) + print("|---|---|---|---|---|", file=out) + print(f"| {status} | " + f"{passed if passed is not None else '-'} | " + f"{failures if failures is not None else '-'} | " + f"{total if total is not None else '-'} | " + f"{duration:.1f}s |" if duration is not None else "- |", + file=out) print("", file=out) - all_failed = [("Unit", n) for n in unit_failed_names] + [("E2E", n) for n in e2e_failed_names] - if all_failed: + if failed_names: print("## Failed Tests\n", file=out) - print("| Suite | Test |", file=out) - print("|---|---|", file=out) - for suite, name in all_failed: - print(f"| {suite} | `{name}` |", file=out) - print("", file=out) - - # Coverage - print("## Coverage\n", file=out) - try: - cov_root = ET.parse("coverage.xml").getroot() - line_rate = float(cov_root.attrib.get("line-rate", 0)) - total_cov = f"{line_rate * 100:.1f}%" - print(f"**Total: {total_cov}**\n", file=out) - print("
", file=out) - print("Per-package breakdown\n", file=out) - print("| Package | Coverage |", file=out) - print("|---|---|", file=out) - for pkg in cov_root.iter("package"): - name = pkg.attrib.get("name", "") - rate = float(pkg.attrib.get("line-rate", 0)) - print(f"| `{name}` | {rate * 100:.1f}% |", file=out) - print("
", file=out) - except FileNotFoundError: - print("> Coverage data not available.", file=out) + print("| Test |", file=out) + print("|---|", file=out) + for name in failed_names: + print(f"| `{name}` |", file=out) EOF diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index e81543c5..acdb86a6 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -9,50 +9,22 @@ permissions: pull-requests: write issues: write id-token: write + actions: read env: PYTHON_VERSION: '3.11' jobs: - # Run quality checks - quality-gates: - name: Quality Gates - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - python-version: ['3.10', '3.11', '3.12', '3.13'] - timeout-minutes: 15 - - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python-version }} - - - name: Install uv - uses: astral-sh/setup-uv@v2 - with: - enable-cache: true - - - name: Install dependencies - run: make dev - - - name: Quality checks - run: make quality-check - - # Release orchestration + # Quality is enforced by CI on PRs via branch protection -- the code that + # reached main has already passed the full matrix. Re-running it here would + # duplicate ~24 min of compute per merge for zero added signal. release-please: name: Release Please runs-on: ubuntu-latest - needs: [quality-gates] - + outputs: release_created: ${{ steps.release.outputs.release_created }} - + steps: - name: Generate GitHub App Token id: app-token @@ -67,30 +39,25 @@ jobs: with: token: ${{ steps.app-token.outputs.token }} - # PyPI publishing pypi-publish: name: PyPI Publish runs-on: ubuntu-latest needs: [release-please] if: ${{ needs.release-please.outputs.release_created == 'true' }} - + environment: name: pypi-production url: https://pypi.org/project/runpod-flash/ - - steps: - - name: Checkout code - uses: actions/checkout@v4 - - name: Set up Python - uses: actions/setup-python@v5 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: python-version: ${{ env.PYTHON_VERSION }} - - - name: Install uv - uses: astral-sh/setup-uv@v2 + - uses: astral-sh/setup-uv@v5 with: enable-cache: true + cache-dependency-glob: pyproject.toml - name: Build package run: make build @@ -101,4 +68,4 @@ jobs: - name: Publish to PyPI uses: pypa/gh-action-pypi-publish@release/v1 with: - verbose: true \ No newline at end of file + verbose: true diff --git a/Makefile b/Makefile index ff9cb101..df30b5c9 100644 --- a/Makefile +++ b/Makefile @@ -12,9 +12,8 @@ help: # Show this help menu @awk 'BEGIN {FS = ":.*# "; printf "%-20s %s\n", "Target", "Description"} /^[a-zA-Z_-]+:.*# / {printf "%-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) @echo "" -dev: # Install development dependencies and package in editable mode +dev: # Install development dependencies (editable install handled by uv sync) uv sync --all-groups - uv pip install -e . update: uv sync --upgrade --all-groups @@ -72,7 +71,7 @@ test-integration: # Run integration tests in parallel (auto-detect cores) test-integration-serial: # Run integration tests serially (for debugging) uv run pytest tests/integration/ -v -m integration -test-coverage: # Run tests with coverage report (parallel by default) +test-coverage: # Run tests with coverage report (parallel non-serial, then serial pass for state isolation) uv run pytest tests/ -v -n auto -m "not serial" --cov=runpod_flash --cov-report=xml uv run pytest tests/ -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing @@ -102,12 +101,13 @@ format-check: # Check code formatting typecheck: # Check types with mypy uv run mypy . -# Quality gates (used in CI) -quality-check: format-check lint test-coverage # Essential quality gate for CI (parallel by default) -quality-check-strict: format-check lint typecheck test-coverage # Strict quality gate with type checking (parallel by default) -quality-check-serial: format-check lint test-coverage-serial # Serial quality gate for debugging +# Quality gates. ci-quality-github is the canonical CI gate; the local aliases +# below run the same checks (with plain output instead of GitHub annotations). +quality-check: ci-quality-github # Essential quality gate (parallel by default) +quality-check-strict: format-check lint typecheck test-coverage # Strict quality gate with type checking +quality-check-serial: ci-quality-github-serial # Serial quality gate for debugging -# GitHub Actions specific targets +# GitHub Actions specific target -- canonical CI quality gate. ci-quality-github: # Quality checks with GitHub Actions formatting (parallel by default) @echo "::group::Code formatting check" uv run ruff format --check . @@ -115,9 +115,11 @@ ci-quality-github: # Quality checks with GitHub Actions formatting (parallel by @echo "::group::Linting" uv run ruff check . --output-format=github @echo "::endgroup::" - @echo "::group::Test suite with coverage" - uv run pytest tests/ --junitxml=pytest-results.xml -v -n auto -m "not serial" --cov=runpod_flash --cov-report=xml --cov-fail-under=0 - uv run pytest tests/ --junitxml=pytest-results.xml -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing + @echo "::group::Test suite with coverage (parallel non-serial)" + uv run pytest tests/ --junitxml=pytest-results-parallel.xml -v -n auto -m "not serial" --cov=runpod_flash --cov-report=xml --cov-fail-under=0 + @echo "::endgroup::" + @echo "::group::Test suite with coverage (serial pass)" + uv run pytest tests/ --junitxml=pytest-results-serial.xml -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing @echo "::endgroup::" ci-quality-github-serial: # Serial quality checks for GitHub Actions (for debugging) From 488506e2c96dc54a1f362f104f9c440e8eff2bfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Mon, 1 Jun 2026 12:55:32 -0700 Subject: [PATCH 2/8] =?UTF-8?q?ci:=20address=20QA=20=E2=80=94=20fix=20skip?= =?UTF-8?q?=20cascade,=20add=20packaging=20guard,=20sentinel=20for=20relea?= =?UTF-8?q?se-please=20bot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses @runpod-Henrik (PASS WITH NITS) and Copilot review on PR #345. Henrik #3 / Copilot inline ci.yml:50 — skip cascade - Previously `pre-check.if` skipped on release-please bot PRs, and `quality-gates needs: [pre-check]` cascaded to skip too, leaving any release-please PR that touched non-metadata files with zero CI signal. - Drop the workflow-level `paths-ignore` (it interacted badly with branch protection's required-check semantics anyway) and instead put explicit `if: !startsWith(github.head_ref, 'release-please--')` on pre-check, quality-gates, packaging-guard, and build. Symmetric, auditable. Henrik #1 + #6 — branch-protection compatibility (release-please bot PRs) - Add `release-please-sentinel` job with matching matrix + name "Quality Gates" so it produces the same "Quality Gates (3.10)"…"(3.13)" check names that branch protection requires, but auto-passes. Branch protection on main can now require those checks without admins having to either loosen the rule or hand-craft an external sentinel workflow. Henrik #2 — PR-time wheel build is dropped - Reintroduce build on PRs but gated by `packaging-guard` job that inspects `git diff` against the PR base. Build runs when: - pyproject.toml / Makefile / MANIFEST.in / scripts/validate-wheel.sh changed, OR - any new non-Python file was added under src/ (data files need explicit package-data inclusion or they're silently dropped). - Push to main always builds. Henrik #4 — `make dev` entry-point sanity check - Verified locally: `rm -rf .venv && make dev && .venv/bin/flash --version` prints "Runpod Flash CLI v1.16.0". `uv sync` registers the entry point for the workspace project; the dropped `uv pip install -e .` was redundant. No code change needed. Henrik #5 / Copilot Makefile:108 — misleading "plain output" comment - Rewrite the comment block above `quality-check`. The aliases do NOT produce plain output -- local invocations see GitHub Actions annotation markers (::group::, --output-format=github) which are cosmetically noisy but functionally inert. Comment now reflects this. Henrik misc nit — e2e.yml `unit-tests` job removal - Add a leading comment on the `e2e` job explaining why the duplicate unit-tests job was dropped, so a future maintainer doesn't re-add it thinking it was lost by accident. https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 77 ++++++++++++++++++++++++++++++++++----- .github/workflows/e2e.yml | 5 +++ Makefile | 6 ++- 3 files changed, 76 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d484a071..9b8391f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,11 +3,6 @@ name: CI on: pull_request: branches: [main] - # release-please bot PRs only touch CHANGELOG / version manifest -- skip - # the full matrix; release-please.yml will exercise publishing on merge. - paths-ignore: - - 'CHANGELOG.md' - - '.release-please-manifest.json' push: branches: [main] @@ -26,6 +21,25 @@ env: PYTHON_VERSION: '3.11' jobs: + # Sentinel for release-please bot PRs (head_ref begins with release-please--). + # Those PRs only contain CHANGELOG.md + .release-please-manifest.json updates + # generated from already-merged commits, so re-running the matrix would be + # pure waste -- but branch protection still requires the "Quality Gates (X)" + # check names to be green. This job mirrors that matrix and trivially passes, + # so the bot's PRs can merge without disabling required checks. + release-please-sentinel: + name: Quality Gates + runs-on: ubuntu-latest + if: ${{ github.event_name == 'pull_request' && startsWith(github.head_ref, 'release-please--') }} + strategy: + matrix: + python-version: ['3.10', '3.11', '3.12', '3.13'] + steps: + - name: Auto-pass for release-please bot PR + run: | + echo "Release-please bot PR: CHANGELOG/manifest only." + echo "Underlying commits were validated by ci.yml when merged to main." + # Fast-fail formatting/lint check. ~10s, no project install. Catches the # common case (ruff format/check fail) before paying for the full matrix. pre-check: @@ -48,6 +62,11 @@ jobs: name: Quality Gates runs-on: ubuntu-latest needs: [pre-check] + # Explicit `if:` instead of relying on needs cascade. If pre-check is + # skipped (release-please bot path), this job would also skip via `needs` + # default semantics, masking the intent. Being explicit also makes the + # symmetry with the sentinel job above easy to audit. + if: ${{ !startsWith(github.head_ref, 'release-please--') }} strategy: fail-fast: false matrix: @@ -82,14 +101,52 @@ jobs: name: test-results-${{ matrix.python-version }} path: pytest-results-*.xml - # Validate the wheel actually builds. Only on push:main since release-please.yml - # builds + publishes from the same SHA; a PR-time wheel build would add ~1 min - # for no extra signal beyond quality-gates. + # Decide whether to run the wheel build on this run. Push to main always + # builds; pull requests only build when packaging-relevant files changed + # (validate-wheel.sh catches regressions that pytest can't, e.g. stale + # package-data globs, missing MANIFEST entries, dropped non-Python data + # files in the wheel). + packaging-guard: + name: Packaging guard + runs-on: ubuntu-latest + if: ${{ !startsWith(github.head_ref, 'release-please--') }} + outputs: + should_build: ${{ steps.check.outputs.should_build }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - id: check + env: + EVENT_NAME: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" = "push" ]; then + echo "should_build=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + CHANGED="$(git diff --name-only "origin/${BASE_REF}...HEAD")" + echo "Changed files in this PR:" + echo "$CHANGED" + # Files that affect what ends up in the wheel. + if echo "$CHANGED" | grep -qE '^(pyproject\.toml|Makefile|MANIFEST\.in|scripts/validate-wheel\.sh)$'; then + echo "should_build=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + # New non-Python files anywhere under src/ -- these need explicit + # package-data inclusion or they'll be silently dropped from the wheel. + if git diff --name-only --diff-filter=A "origin/${BASE_REF}...HEAD" | grep -E '^src/.+' | grep -qv '\.py$'; then + echo "should_build=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "should_build=false" >> "$GITHUB_OUTPUT" + build: name: Build Package runs-on: ubuntu-latest - needs: [quality-gates] - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + needs: [quality-gates, packaging-guard] + if: ${{ needs.packaging-guard.outputs.should_build == 'true' }} timeout-minutes: 5 steps: diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 2c78be57..0ca50eed 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -15,6 +15,11 @@ env: PYTHON_VERSION: '3.11' jobs: + # Note: there's intentionally no unit-tests job here. ci.yml already runs the + # full quality matrix on every PR and push:main; duplicating it under + # workflow_dispatch would just be a third copy of the same install + pytest + # setup. If you need unit results outside of CI, run ci.yml manually or + # rerun a previous run. e2e: name: E2E runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index df30b5c9..4d8d097c 100644 --- a/Makefile +++ b/Makefile @@ -101,8 +101,10 @@ format-check: # Check code formatting typecheck: # Check types with mypy uv run mypy . -# Quality gates. ci-quality-github is the canonical CI gate; the local aliases -# below run the same checks (with plain output instead of GitHub annotations). +# Quality gates. ci-quality-github is the canonical CI gate; local aliases run +# the same recipe. Local invocations will see GitHub Actions annotation markers +# (::group::, --output-format=github) in their output -- these are inert +# outside Actions but cosmetically noisy. Single source of truth wins. quality-check: ci-quality-github # Essential quality gate (parallel by default) quality-check-strict: format-check lint typecheck test-coverage # Strict quality gate with type checking quality-check-serial: ci-quality-github-serial # Serial quality gate for debugging From 64bc5fb953b1cbc828b4160a0ccb203710c95d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Mon, 1 Jun 2026 20:59:45 -0700 Subject: [PATCH 3/8] ci: close sentinel-bypass hole and pin pre-check ruff to lockfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot's second review on PR #345. Copilot ci.yml:42 — sentinel-bypass vulnerability - Previous sentinel gated only on `startsWith(head_ref, 'release-please--')`. A contributor could open a PR from any branch named with that prefix and inherit the sentinel's auto-passes for "Quality Gates (3.10..3.13)" while pre-check / quality-gates / build all skipped via the same `if:` guard. Net: CI fully bypassed, branch protection satisfied, code unreviewed. - Replace the three independent `if:` gates with a single `guard` job that inspects the actual PR diff and classifies the run as either `release-please-bypass` (branch prefix AND diff is exactly CHANGELOG.md + .release-please-manifest.json) or `full`. `release-please-sentinel` only fires when classification is bypass; pre-check / quality-gates / build fire only when classification is full. A contributor naming a branch release-please-- but changing code falls through to full CI. - Also folds the previous packaging-guard logic into the same job; build reads `should_build` from the unified guard output. Copilot ci.yml:59 — ruff version drift in pre-check - `uvx ruff …` installed the latest ruff at runtime, which can drift from the locked version (uv.lock pins ruff 0.14.14) used by `make ci-quality-github`. That produces flaky "passes locally / fails CI" (or vice versa) verdicts when ruff cuts a release between runs. - Pre-check now runs `uv sync --only-group dev --frozen` then `uv run ruff …`. Pulls the exact pinned version; warm-cache install is ~1s, cold is a few seconds. Fast-fail premise preserved; determinism gained. https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 155 +++++++++++++++++++++++---------------- 1 file changed, 93 insertions(+), 62 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b8391f7..8ef9491b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,16 +21,85 @@ env: PYTHON_VERSION: '3.11' jobs: - # Sentinel for release-please bot PRs (head_ref begins with release-please--). - # Those PRs only contain CHANGELOG.md + .release-please-manifest.json updates - # generated from already-merged commits, so re-running the matrix would be - # pure waste -- but branch protection still requires the "Quality Gates (X)" - # check names to be green. This job mirrors that matrix and trivially passes, - # so the bot's PRs can merge without disabling required checks. + # Single source of truth for "what mode should this run be in?". Inspects the + # PR diff and outputs: + # mode = release-please-bypass | full + # should_build = true | false + # The classification is content-based, not just branch-name-based -- that + # closes the CI-bypass hole where a contributor opens a PR from a branch + # named release-please-- and inherits sentinel passes for free. + guard: + name: Guard + runs-on: ubuntu-latest + timeout-minutes: 2 + outputs: + mode: ${{ steps.classify.outputs.mode }} + should_build: ${{ steps.classify.outputs.should_build }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - id: classify + env: + EVENT_NAME: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + HEAD_REF: ${{ github.head_ref }} + run: | + set -euo pipefail + + # Push to main: always run full CI; always build. + if [ "$EVENT_NAME" = "push" ]; then + echo "mode=full" >> "$GITHUB_OUTPUT" + echo "should_build=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + CHANGED="$(git diff --name-only "origin/${BASE_REF}...HEAD")" + echo "Changed files in this PR:" + echo "$CHANGED" + + # release-please bypass: branch prefix AND diff is metadata-only. + # Both conditions required -- a contributor naming a branch + # release-please-- but changing source code falls through to full CI. + if [[ "$HEAD_REF" == release-please--* ]]; then + ALLOWED='^(CHANGELOG\.md|\.release-please-manifest\.json)$' + UNEXPECTED="$(echo "$CHANGED" | grep -vE "$ALLOWED" || true)" + if [ -z "$UNEXPECTED" ]; then + echo "Classification: release-please bot PR (metadata only)." + echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" + echo "should_build=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "release-please-- branch contains non-metadata files; falling through to full CI." + echo "Unexpected files:" + echo "$UNEXPECTED" + fi + + # Default: full CI. Build only when packaging-relevant files changed + # or a new non-Python file is added under src/ (data files need + # explicit package-data inclusion or they're silently dropped from + # the wheel). + echo "mode=full" >> "$GITHUB_OUTPUT" + if echo "$CHANGED" | grep -qE '^(pyproject\.toml|Makefile|MANIFEST\.in|scripts/validate-wheel\.sh)$'; then + echo "should_build=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + if git diff --name-only --diff-filter=A "origin/${BASE_REF}...HEAD" | grep -E '^src/.+' | grep -qv '\.py$'; then + echo "should_build=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "should_build=false" >> "$GITHUB_OUTPUT" + + # Sentinel for verified release-please bot PRs. The classification in `guard` + # guarantees this only fires when the diff is genuinely metadata-only, so + # it's safe to short-circuit the full matrix and emit matching "Quality + # Gates (X)" check names to satisfy branch protection. release-please-sentinel: name: Quality Gates runs-on: ubuntu-latest - if: ${{ github.event_name == 'pull_request' && startsWith(github.head_ref, 'release-please--') }} + needs: [guard] + if: ${{ needs.guard.outputs.mode == 'release-please-bypass' }} strategy: matrix: python-version: ['3.10', '3.11', '3.12', '3.13'] @@ -40,33 +109,36 @@ jobs: echo "Release-please bot PR: CHANGELOG/manifest only." echo "Underlying commits were validated by ci.yml when merged to main." - # Fast-fail formatting/lint check. ~10s, no project install. Catches the - # common case (ruff format/check fail) before paying for the full matrix. + # Fast-fail formatting/lint check. Installs only the dev dependency group + # from the lockfile (warm-cache install is ~1s), then runs ruff via + # `uv run` so we get the exact pinned ruff version that `make + # ci-quality-github` uses. Avoids `uvx ruff` -- that pulls the latest + # ruff, which can drift from the lock and produce CI verdicts that don't + # match local `make quality-check`. pre-check: name: Pre-check (format + lint) runs-on: ubuntu-latest + needs: [guard] + if: ${{ needs.guard.outputs.mode == 'full' }} timeout-minutes: 3 - if: ${{ !startsWith(github.head_ref, 'release-please--') }} steps: - uses: actions/checkout@v4 - uses: astral-sh/setup-uv@v5 with: enable-cache: true - cache-dependency-glob: pyproject.toml + cache-dependency-glob: uv.lock + - name: Install dev group (frozen, no project) + run: uv sync --only-group dev --frozen - name: Ruff format - run: uvx ruff format --check . + run: uv run ruff format --check . - name: Ruff lint - run: uvx ruff check . --output-format=github + run: uv run ruff check . --output-format=github quality-gates: name: Quality Gates runs-on: ubuntu-latest - needs: [pre-check] - # Explicit `if:` instead of relying on needs cascade. If pre-check is - # skipped (release-please bot path), this job would also skip via `needs` - # default semantics, masking the intent. Being explicit also makes the - # symmetry with the sentinel job above easy to audit. - if: ${{ !startsWith(github.head_ref, 'release-please--') }} + needs: [guard, pre-check] + if: ${{ needs.guard.outputs.mode == 'full' }} strategy: fail-fast: false matrix: @@ -101,52 +173,11 @@ jobs: name: test-results-${{ matrix.python-version }} path: pytest-results-*.xml - # Decide whether to run the wheel build on this run. Push to main always - # builds; pull requests only build when packaging-relevant files changed - # (validate-wheel.sh catches regressions that pytest can't, e.g. stale - # package-data globs, missing MANIFEST entries, dropped non-Python data - # files in the wheel). - packaging-guard: - name: Packaging guard - runs-on: ubuntu-latest - if: ${{ !startsWith(github.head_ref, 'release-please--') }} - outputs: - should_build: ${{ steps.check.outputs.should_build }} - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - id: check - env: - EVENT_NAME: ${{ github.event_name }} - BASE_REF: ${{ github.base_ref }} - run: | - set -euo pipefail - if [ "$EVENT_NAME" = "push" ]; then - echo "should_build=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - CHANGED="$(git diff --name-only "origin/${BASE_REF}...HEAD")" - echo "Changed files in this PR:" - echo "$CHANGED" - # Files that affect what ends up in the wheel. - if echo "$CHANGED" | grep -qE '^(pyproject\.toml|Makefile|MANIFEST\.in|scripts/validate-wheel\.sh)$'; then - echo "should_build=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - # New non-Python files anywhere under src/ -- these need explicit - # package-data inclusion or they'll be silently dropped from the wheel. - if git diff --name-only --diff-filter=A "origin/${BASE_REF}...HEAD" | grep -E '^src/.+' | grep -qv '\.py$'; then - echo "should_build=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - echo "should_build=false" >> "$GITHUB_OUTPUT" - build: name: Build Package runs-on: ubuntu-latest - needs: [quality-gates, packaging-guard] - if: ${{ needs.packaging-guard.outputs.should_build == 'true' }} + needs: [guard, quality-gates] + if: ${{ needs.guard.outputs.mode == 'full' && needs.guard.outputs.should_build == 'true' }} timeout-minutes: 5 steps: From c548e526b64a9a4b44b54d6a43c9fe4d2203d9f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Tue, 2 Jun 2026 09:44:13 -0700 Subject: [PATCH 4/8] ci: replace release-please sentinel with single Validation aggregator Borrowed from runpod-python (ci.yml:105) -- one required check that inspects upstream `needs.*.result` and treats "success" or "skipped" as green. Much cleaner than the previous sentinel-with-matching-matrix-names hack: - Drops `release-please-sentinel` job entirely. No more duplicate `Quality Gates` display name. No more "skipped Quality Gates" cosmetic row alongside the real four matrix legs on every regular PR. - For release-please bot bypass runs: guard classifies bypass -> pre-check / quality-gates / build all skip -> `validation` aggregates ("skipped" treated as ok) -> green. - For regular PRs: everything runs -> `validation` aggregates -> green. - For any genuinely failed or cancelled job: `validation` exits 1 -> branch protection blocks merge. Future matrix changes (add 3.14, drop 3.10) or new jobs (security scans, etc.) no longer require touching branch protection -- only the `needs:` list and the results array in this aggregator. Requires branch protection on `main` to be reconfigured before merge: - Remove required checks: `CI / Quality Gates (3.10)`, `(3.11)`, `(3.12)`, `(3.13)` - Add required check: `CI / Validation` https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 55 +++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ef9491b..ab4467d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,24 +91,6 @@ jobs: fi echo "should_build=false" >> "$GITHUB_OUTPUT" - # Sentinel for verified release-please bot PRs. The classification in `guard` - # guarantees this only fires when the diff is genuinely metadata-only, so - # it's safe to short-circuit the full matrix and emit matching "Quality - # Gates (X)" check names to satisfy branch protection. - release-please-sentinel: - name: Quality Gates - runs-on: ubuntu-latest - needs: [guard] - if: ${{ needs.guard.outputs.mode == 'release-please-bypass' }} - strategy: - matrix: - python-version: ['3.10', '3.11', '3.12', '3.13'] - steps: - - name: Auto-pass for release-please bot PR - run: | - echo "Release-please bot PR: CHANGELOG/manifest only." - echo "Underlying commits were validated by ci.yml when merged to main." - # Fast-fail formatting/lint check. Installs only the dev dependency group # from the lockfile (warm-cache install is ~1s), then runs ruff via # `uv run` so we get the exact pinned ruff version that `make @@ -198,3 +180,40 @@ jobs: - name: Validate wheel packaging run: ./scripts/validate-wheel.sh + + # Single aggregator. Branch protection on `main` should require ONLY this + # check ("CI / Validation"). This job runs unconditionally (`if: always()`) + # and treats "skipped" as success -- so release-please-bypass runs (where + # pre-check / quality-gates / build are all deliberately skipped by the + # guard) pass cleanly. Anything that genuinely failed or was cancelled + # upstream flips this to red. + # + # Adding or removing upstream jobs (new Python version, new security scan, + # etc.) no longer requires a branch-protection update: just include the new + # job in `needs:` and the results array. + validation: + name: Validation + runs-on: ubuntu-latest + needs: [guard, pre-check, quality-gates, build] + if: always() + timeout-minutes: 1 + steps: + - name: Aggregate upstream results + env: + GUARD: ${{ needs.guard.result }} + PRE_CHECK: ${{ needs.pre-check.result }} + QUALITY_GATES: ${{ needs.quality-gates.result }} + BUILD: ${{ needs.build.result }} + run: | + set -euo pipefail + echo "guard=$GUARD" + echo "pre-check=$PRE_CHECK" + echo "quality-gates=$QUALITY_GATES" + echo "build=$BUILD" + for r in "$GUARD" "$PRE_CHECK" "$QUALITY_GATES" "$BUILD"; do + if [ "$r" != "success" ] && [ "$r" != "skipped" ]; then + echo "::error::Upstream job failed or was cancelled (got: $r)" + exit 1 + fi + done + echo "All upstream jobs succeeded or were intentionally skipped." From 1a6a0b695430c7923db80e2fe75831af212c6362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Tue, 2 Jun 2026 10:04:16 -0700 Subject: [PATCH 5/8] ci: classify metadata-only push:main commits as release-please-bypass Closes the last remaining redundancy in the lifecycle. When release-please's release PR is merged, the resulting commit on main only changes CHANGELOG.md and .release-please-manifest.json -- the same code was already validated when the underlying feature PRs merged. Previously ci.yml ran the full 4-version matrix + build on those merge commits (~7 min compute per release). Now `guard` classifies push:main by diff: - Metadata-only diff -> release-please-bypass -> pre-check / quality-gates / build all skip, validation aggregates "skipped" as success. - Any non-metadata change (feature merges, admin direct pushes) -> full. release-please.yml's pypi-publish builds its own wheel before publishing, so skipping ci.yml's build on release commits doesn't blind us to wheel regressions. https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab4467d0..f10f36c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,8 +48,28 @@ jobs: run: | set -euo pipefail - # Push to main: always run full CI; always build. + # Push to main: classify by diff against the previous commit. + # When release-please's release PR is merged, the resulting commit on + # main only changes CHANGELOG.md + .release-please-manifest.json -- + # the same code that was already validated when the underlying + # feature PRs merged. Re-running the full matrix would be pure waste, + # so we bypass; release-please.yml's pypi-publish builds its own + # wheel before publishing, so we don't need ci.yml's build either. + # Any other push:main (feature merges, direct admin pushes) gets the + # full matrix and build. if [ "$EVENT_NAME" = "push" ]; then + CHANGED="$(git diff --name-only HEAD~1...HEAD)" + echo "Changed files in this push:" + echo "$CHANGED" + ALLOWED='^(CHANGELOG\.md|\.release-please-manifest\.json)$' + UNEXPECTED="$(echo "$CHANGED" | grep -vE "$ALLOWED" || true)" + if [ -n "$CHANGED" ] && [ -z "$UNEXPECTED" ]; then + echo "Classification: release-please release commit on main (metadata only)." + echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" + echo "should_build=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "Classification: regular push to main." echo "mode=full" >> "$GITHUB_OUTPUT" echo "should_build=true" >> "$GITHUB_OUTPUT" exit 0 From 97920a6fe5fa8af2d67224dde40482f927210eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Wed, 3 Jun 2026 01:17:40 -0700 Subject: [PATCH 6/8] ci: align bypass classifier with release-please-config; use uv.lock for cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot's third review on PR #345. Copilot ci.yml:66 + ci.yml:88 — bypass missing release-please-managed files - release-please-config.json declares release-type=python (auto-bumps pyproject.toml) and extra-files=src/runpod_flash/__init__.py. Real bot PRs touch 4 files, not 2. Previous ALLOWED regex missed pyproject.toml and __init__.py, so every real release-please bot PR was misclassified as `full`. Entire bypass optimization was broken for the actual case it was designed for. - Extend ALLOWED to all four files. Add a comment pointing back to release-please-config.json so the regex stays in sync. - Add defense-in-depth: PR classifier now also requires github.event.pull_request.user.login == 'runpod-release-please-bot[bot]' (the GitHub App account). Branch prefix + bot author + file content all required. A contributor spoofing the branch name fails the author check; a contributor with write access to the bot would already be trusted. - Push:main classifier keeps content-only check (admin can squash-merge the bot's PR by hand, so actor isn't a reliable signal on push). Copilot ci.yml:164 + ci.yml:193 + release-please.yml:61 + e2e.yml:39 — setup-uv cache keyed on pyproject.toml - uv sync is lockfile-driven; pyproject.toml-only cache key means uv.lock changes (most common dependency update) don't invalidate the cache, causing stale-deps cache hits. - Switch cache-dependency-glob to uv.lock everywhere. pre-check already used uv.lock; this aligns the rest. Copilot Makefile:107 — misleading "aliases" comment - quality-check-strict is NOT an alias for ci-quality-github -- it runs a different set (adds typecheck, plain output, no junit). Comment block rewritten to enumerate each target individually so maintainers don't conflate them. https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 62 +++++++++++++++++++--------- .github/workflows/e2e.yml | 2 +- .github/workflows/release-please.yml | 2 +- Makefile | 14 +++++-- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f10f36c0..211af471 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,26 +45,42 @@ jobs: EVENT_NAME: ${{ github.event_name }} BASE_REF: ${{ github.base_ref }} HEAD_REF: ${{ github.head_ref }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} run: | set -euo pipefail - # Push to main: classify by diff against the previous commit. - # When release-please's release PR is merged, the resulting commit on - # main only changes CHANGELOG.md + .release-please-manifest.json -- - # the same code that was already validated when the underlying - # feature PRs merged. Re-running the full matrix would be pure waste, - # so we bypass; release-please.yml's pypi-publish builds its own - # wheel before publishing, so we don't need ci.yml's build either. - # Any other push:main (feature merges, direct admin pushes) gets the - # full matrix and build. + # Files release-please touches when cutting a release for this repo. + # Keep in sync with release-please-config.json: the .packages."." entry + # determines which files are bumped. Today (release-type=python + + # extra-files=src/runpod_flash/__init__.py) those are: + # - CHANGELOG.md (always) + # - .release-please-manifest.json (always) + # - pyproject.toml (release-type=python) + # - src/runpod_flash/__init__.py (extra-files) + # If you change release-please-config.json, update this regex too. + ALLOWED='^(CHANGELOG\.md|\.release-please-manifest\.json|pyproject\.toml|src/runpod_flash/__init__\.py)$' + + # Identity check: real release-please bot PRs are authored by the + # GitHub App. Used as a defense-in-depth layer on top of the branch + # prefix + diff content check. + BOT_AUTHOR='runpod-release-please-bot[bot]' + + # Push to main: classify by diff against the previous commit. When + # release-please's release PR is squash-merged, the resulting commit + # on main only touches the files in ALLOWED -- same code, just + # version-bumped. Re-running the full matrix would be pure waste, and + # release-please.yml's pypi-publish builds its own wheel before + # publishing, so we don't need ci.yml's build either. Anything else + # (feature merges, direct admin pushes) gets the full matrix. + # On push events the actor isn't a reliable signal (admins can merge + # the bot's PR by hand) -- diff content is the only check. if [ "$EVENT_NAME" = "push" ]; then CHANGED="$(git diff --name-only HEAD~1...HEAD)" echo "Changed files in this push:" echo "$CHANGED" - ALLOWED='^(CHANGELOG\.md|\.release-please-manifest\.json)$' UNEXPECTED="$(echo "$CHANGED" | grep -vE "$ALLOWED" || true)" if [ -n "$CHANGED" ] && [ -z "$UNEXPECTED" ]; then - echo "Classification: release-please release commit on main (metadata only)." + echo "Classification: release-please release commit on main (release-please files only)." echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" echo "should_build=false" >> "$GITHUB_OUTPUT" exit 0 @@ -79,21 +95,27 @@ jobs: echo "Changed files in this PR:" echo "$CHANGED" - # release-please bypass: branch prefix AND diff is metadata-only. - # Both conditions required -- a contributor naming a branch - # release-please-- but changing source code falls through to full CI. - if [[ "$HEAD_REF" == release-please--* ]]; then - ALLOWED='^(CHANGELOG\.md|\.release-please-manifest\.json)$' + # release-please bypass on PR: requires ALL THREE: + # 1. Branch prefix matches `release-please--` + # 2. PR author is the release-please GitHub App + # 3. Diff contains only release-please-managed files + # Any one missing -> fall through to full CI. Together these mean a + # contributor can't fake the bypass by naming a branch + # release-please-- (author check fails) or by spoofing the bot + # account (write-access required to push as the bot is even stronger). + if [[ "$HEAD_REF" == release-please--* ]] && [ "$PR_AUTHOR" = "$BOT_AUTHOR" ]; then UNEXPECTED="$(echo "$CHANGED" | grep -vE "$ALLOWED" || true)" if [ -z "$UNEXPECTED" ]; then - echo "Classification: release-please bot PR (metadata only)." + echo "Classification: release-please bot PR (release-please files only)." echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" echo "should_build=false" >> "$GITHUB_OUTPUT" exit 0 fi - echo "release-please-- branch contains non-metadata files; falling through to full CI." + echo "release-please-- branch from bot contains unexpected files; falling through to full CI." echo "Unexpected files:" echo "$UNEXPECTED" + elif [[ "$HEAD_REF" == release-please--* ]]; then + echo "release-please-- branch but author is '$PR_AUTHOR' (not '$BOT_AUTHOR'); running full CI." fi # Default: full CI. Build only when packaging-relevant files changed @@ -160,7 +182,7 @@ jobs: uses: astral-sh/setup-uv@v5 with: enable-cache: true - cache-dependency-glob: pyproject.toml + cache-dependency-glob: uv.lock - name: Install dependencies run: make dev @@ -190,7 +212,7 @@ jobs: - uses: astral-sh/setup-uv@v5 with: enable-cache: true - cache-dependency-glob: pyproject.toml + cache-dependency-glob: uv.lock - name: Build package run: make build diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 0ca50eed..b008cf9e 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -35,7 +35,7 @@ jobs: - uses: astral-sh/setup-uv@v5 with: enable-cache: true - cache-dependency-glob: pyproject.toml + cache-dependency-glob: uv.lock - name: Install dependencies run: make dev diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index acdb86a6..b9ab1277 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -57,7 +57,7 @@ jobs: - uses: astral-sh/setup-uv@v5 with: enable-cache: true - cache-dependency-glob: pyproject.toml + cache-dependency-glob: uv.lock - name: Build package run: make build diff --git a/Makefile b/Makefile index 4d8d097c..c764d4d4 100644 --- a/Makefile +++ b/Makefile @@ -101,10 +101,16 @@ format-check: # Check code formatting typecheck: # Check types with mypy uv run mypy . -# Quality gates. ci-quality-github is the canonical CI gate; local aliases run -# the same recipe. Local invocations will see GitHub Actions annotation markers -# (::group::, --output-format=github) in their output -- these are inert -# outside Actions but cosmetically noisy. Single source of truth wins. +# Quality gates. +# +# - quality-check / quality-check-serial: aliases for ci-quality-github / +# ci-quality-github-serial. These are the canonical CI gates. When invoked +# locally you'll see GitHub Actions annotation markers (::group::, +# --output-format=github) in the output -- inert outside Actions but +# cosmetically noisy. The alias guarantees local and CI run the same recipe. +# - quality-check-strict: NOT an alias. Adds mypy typecheck on top of the +# plain format/lint/test-coverage targets, with plain output (no annotation +# markers, no JUnit XML). Use when you want stricter local feedback. quality-check: ci-quality-github # Essential quality gate (parallel by default) quality-check-strict: format-check lint typecheck test-coverage # Strict quality gate with type checking quality-check-serial: ci-quality-github-serial # Serial quality gate for debugging From e467bc3d9ece69bc9e22c046cfe8f3a3c83fb726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Wed, 3 Jun 2026 21:31:56 -0700 Subject: [PATCH 7/8] ci: derive bypass file list from release-please-config; pre-check no-sync; catch modified data files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Henrik's delta review on PR #345. Henrik #6 — ALLOWED was hand-synced with release-please-config.json - Previous regex hardcoded the four files release-please touches today. Asymmetric failure mode is safe (out-of-sync -> bypass doesn't fire -> full CI runs) but the next person to add an `extra-files` entry to release-please-config.json would silently break the bypass for bot PRs. - Derive the file list dynamically via jq from release-please-config.json's `.packages["."]["extra-files"]`, append the three implicit files (CHANGELOG.md, manifest, pyproject.toml from release-type=python). Use `grep -Fxvf <(allowed_files)` for fixed-string exact-line matching -- removes the regex-escaping concern entirely. Henrik #7 — `uv run` undid the dev-only sync - `uv run` defaults to syncing the environment before each invocation, so the steps after `uv sync --only-group dev --frozen` would pull in the full project on each ruff call -- defeating the "no project install" premise of pre-check. - Add `--no-sync` to both ruff invocations. ruff runs from the existing dev-only venv directly; pre-check stays fast. Henrik #8 — `--diff-filter=A` missed modified non-Python files - Edits to existing packaged data files (e.g. future tweaks to runpod_flash/rules/AGENTS.md) wouldn't trigger should_build, so validate-wheel.sh wouldn't smoke-test the modified content. Packaging metadata wouldn't change, but the wheel content would. - Switch to `--diff-filter=AM` (Added or Modified). Cheap and correct. Henrik #9 (fetch-depth: 0 over-broad) intentionally not addressed -- Henrik flagged it as "not a real cost issue." Henrik #4 (make dev entry-point) re-verified locally: `rm -rf .venv && make dev && .venv/bin/flash --version` -> Runpod Flash CLI v1.16.0. https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 63 ++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 211af471..bb1836c5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,25 +49,37 @@ jobs: run: | set -euo pipefail - # Files release-please touches when cutting a release for this repo. - # Keep in sync with release-please-config.json: the .packages."." entry - # determines which files are bumped. Today (release-type=python + - # extra-files=src/runpod_flash/__init__.py) those are: - # - CHANGELOG.md (always) - # - .release-please-manifest.json (always) - # - pyproject.toml (release-type=python) - # - src/runpod_flash/__init__.py (extra-files) - # If you change release-please-config.json, update this regex too. - ALLOWED='^(CHANGELOG\.md|\.release-please-manifest\.json|pyproject\.toml|src/runpod_flash/__init__\.py)$' + # Derive the list of release-please-managed files from + # release-please-config.json so this stays in sync automatically. + # Implicit files that release-please always touches: + # - CHANGELOG.md + # - .release-please-manifest.json + # - pyproject.toml (release-type=python bumps the version field) + # Anything declared in `.packages."."."extra-files"` is appended. + # If release-type changes (currently "python"), audit this list. + allowed_files() { + printf '%s\n' \ + 'CHANGELOG.md' \ + '.release-please-manifest.json' \ + 'pyproject.toml' + jq -r '.packages["."]["extra-files"][]?' release-please-config.json + } + + # Use fixed-string exact-line matching (`grep -Fxvf`) instead of a + # hand-built regex -- removes the need to escape filenames at all. + is_unexpected() { + # $1 = newline-separated changed files + echo "$1" | grep -Fxvf <(allowed_files) || true + } # Identity check: real release-please bot PRs are authored by the - # GitHub App. Used as a defense-in-depth layer on top of the branch - # prefix + diff content check. + # GitHub App. Used as defense-in-depth alongside the branch prefix + + # diff content check. BOT_AUTHOR='runpod-release-please-bot[bot]' # Push to main: classify by diff against the previous commit. When # release-please's release PR is squash-merged, the resulting commit - # on main only touches the files in ALLOWED -- same code, just + # on main only touches the allowed-files set -- same code, just # version-bumped. Re-running the full matrix would be pure waste, and # release-please.yml's pypi-publish builds its own wheel before # publishing, so we don't need ci.yml's build either. Anything else @@ -78,7 +90,7 @@ jobs: CHANGED="$(git diff --name-only HEAD~1...HEAD)" echo "Changed files in this push:" echo "$CHANGED" - UNEXPECTED="$(echo "$CHANGED" | grep -vE "$ALLOWED" || true)" + UNEXPECTED="$(is_unexpected "$CHANGED")" if [ -n "$CHANGED" ] && [ -z "$UNEXPECTED" ]; then echo "Classification: release-please release commit on main (release-please files only)." echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" @@ -99,12 +111,9 @@ jobs: # 1. Branch prefix matches `release-please--` # 2. PR author is the release-please GitHub App # 3. Diff contains only release-please-managed files - # Any one missing -> fall through to full CI. Together these mean a - # contributor can't fake the bypass by naming a branch - # release-please-- (author check fails) or by spoofing the bot - # account (write-access required to push as the bot is even stronger). + # Any one missing -> fall through to full CI. if [[ "$HEAD_REF" == release-please--* ]] && [ "$PR_AUTHOR" = "$BOT_AUTHOR" ]; then - UNEXPECTED="$(echo "$CHANGED" | grep -vE "$ALLOWED" || true)" + UNEXPECTED="$(is_unexpected "$CHANGED")" if [ -z "$UNEXPECTED" ]; then echo "Classification: release-please bot PR (release-please files only)." echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" @@ -119,15 +128,16 @@ jobs: fi # Default: full CI. Build only when packaging-relevant files changed - # or a new non-Python file is added under src/ (data files need - # explicit package-data inclusion or they're silently dropped from - # the wheel). + # or a non-Python file under src/ is added or modified (data files + # need explicit package-data inclusion or they're silently dropped + # from the wheel; content edits to existing data files should also + # be exercised by validate-wheel.sh). echo "mode=full" >> "$GITHUB_OUTPUT" if echo "$CHANGED" | grep -qE '^(pyproject\.toml|Makefile|MANIFEST\.in|scripts/validate-wheel\.sh)$'; then echo "should_build=true" >> "$GITHUB_OUTPUT" exit 0 fi - if git diff --name-only --diff-filter=A "origin/${BASE_REF}...HEAD" | grep -E '^src/.+' | grep -qv '\.py$'; then + if git diff --name-only --diff-filter=AM "origin/${BASE_REF}...HEAD" | grep -E '^src/.+' | grep -qv '\.py$'; then echo "should_build=true" >> "$GITHUB_OUTPUT" exit 0 fi @@ -153,10 +163,13 @@ jobs: cache-dependency-glob: uv.lock - name: Install dev group (frozen, no project) run: uv sync --only-group dev --frozen + # `uv run` defaults to syncing the environment before each invocation, + # which would undo the dev-only install above and pull in the full + # project. `--no-sync` runs ruff from the existing .venv directly. - name: Ruff format - run: uv run ruff format --check . + run: uv run --no-sync ruff format --check . - name: Ruff lint - run: uv run ruff check . --output-format=github + run: uv run --no-sync ruff check . --output-format=github quality-gates: name: Quality Gates From 722b38c149b5bc11f651fa06e5a1001b2e06f241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Thu, 4 Jun 2026 06:39:57 -0700 Subject: [PATCH 8/8] ci: content-shape bypass check, conditional build on push, e2e summary table fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot fourth review on PR #345. Copilot ci.yml:98 — pyproject.toml in ALLOWED could let non-version edits slip through the bypass - A push:main touching only the metadata filenames could smuggle non-version edits to pyproject.toml (deps, ruff config, scripts) since the previous check was filename-only. Push:main can't rely on actor identity (admins can squash-merge by hand), so the file-list check was the only gate. - Add `verify_version_shape` helper: enforces that any pyproject.toml change is a single `version = "..."` line, and any src/runpod_flash/__init__.py change is a single `__version__ = "..."` line. Applies to both push:main and PR bypass paths. Tested locally against simulated bot and malicious diffs. Copilot ci.yml:103 — `should_build=true` unconditional on push:main - Previous code only computed should_build content-based on PR full-mode; push:main non-bypass always built. Contradicted the documented intent and cost ~1 min per feature merge. - Factor the content heuristic into `compute_should_build`; use it for both push:main full and PR full paths. Same allow-list (pyproject.toml, Makefile, MANIFEST.in, validate-wheel.sh) plus AM-filter on non-Python files under src/. Copilot e2e.yml:129 — malformed markdown table when JUnit file missing - Python ternary `... if duration is not None else "- |"` had the wrong scope: when duration was None, the *entire* print expression became the string `"- |"`, breaking the table. - Extract `_num()` and `_dur()` helpers that placehold "-" per field; build the row in one f-string. Table stays valid when the suite didn't run. Copilot Makefile:116 (NOT changed) — claim was that aliasing quality-check to ci-quality-github drops `--cov-fail-under=65` enforcement. Verified locally: the serial pass in ci-quality-github inherits --cov-fail-under=65 from pyproject.toml's addopts (only the parallel pass overrides to 0). Local `make quality-check` still hits the 65% gate at the end of the serial pass; observed "Required test coverage of 65% reached. Total coverage: 85.96%" on this branch. Will reply on the thread, not change code. https://linear.app/runpod/issue/AE-3161 --- .github/workflows/ci.yml | 109 +++++++++++++++++++++++++++----------- .github/workflows/e2e.yml | 11 ++-- 2 files changed, 85 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bb1836c5..522ff5ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,6 +72,64 @@ jobs: echo "$1" | grep -Fxvf <(allowed_files) || true } + # Content-shape check: a metadata-only file list isn't enough on + # push:main, where actor isn't a reliable bypass signal. Verify the + # diffs themselves are version-bump-shaped: pyproject.toml only + # changes its `version = "..."` line, src/runpod_flash/__init__.py + # only changes its `__version__ = "..."` line. Defeats the + # exploitation path where a direct push touches only the metadata + # filenames but smuggles non-version edits (deps, ruff config, + # scripts) into pyproject.toml. + # $1 = git diff base (HEAD~1 for push, origin/BASE_REF for PR). + verify_version_shape() { + local base="$1" + local bad + if git diff --name-only "${base}...HEAD" -- pyproject.toml | grep -q .; then + bad="$(git diff "${base}...HEAD" -- pyproject.toml \ + | grep -E '^[-+]' \ + | grep -vE '^(---|\+\+\+)' \ + | grep -vE '^[-+]version[[:space:]]*=[[:space:]]*"[^"]+"[[:space:]]*$' || true)" + if [ -n "$bad" ]; then + echo "Bypass rejected: pyproject.toml diff touches non-version lines:" + echo "$bad" + return 1 + fi + fi + if git diff --name-only "${base}...HEAD" -- src/runpod_flash/__init__.py | grep -q .; then + bad="$(git diff "${base}...HEAD" -- src/runpod_flash/__init__.py \ + | grep -E '^[-+]' \ + | grep -vE '^(---|\+\+\+)' \ + | grep -vE '^[-+]__version__[[:space:]]*=[[:space:]]*"[^"]+"[[:space:]]*$' || true)" + if [ -n "$bad" ]; then + echo "Bypass rejected: src/runpod_flash/__init__.py diff touches non-__version__ lines:" + echo "$bad" + return 1 + fi + fi + return 0 + } + + # Build-trigger heuristic shared between push:main and PR full-mode. + # Build runs when packaging-relevant files change OR a non-Python + # file under src/ is added or modified (data files need explicit + # package-data inclusion; content edits should be smoke-tested by + # validate-wheel.sh). + # $1 = git diff base. + compute_should_build() { + local base="$1" + local changed + changed="$(git diff --name-only "${base}...HEAD")" + if echo "$changed" | grep -qE '^(pyproject\.toml|Makefile|MANIFEST\.in|scripts/validate-wheel\.sh)$'; then + echo "true" + return + fi + if git diff --name-only --diff-filter=AM "${base}...HEAD" | grep -E '^src/.+' | grep -qv '\.py$'; then + echo "true" + return + fi + echo "false" + } + # Identity check: real release-please bot PRs are authored by the # GitHub App. Used as defense-in-depth alongside the branch prefix + # diff content check. @@ -79,27 +137,27 @@ jobs: # Push to main: classify by diff against the previous commit. When # release-please's release PR is squash-merged, the resulting commit - # on main only touches the allowed-files set -- same code, just - # version-bumped. Re-running the full matrix would be pure waste, and - # release-please.yml's pypi-publish builds its own wheel before - # publishing, so we don't need ci.yml's build either. Anything else - # (feature merges, direct admin pushes) gets the full matrix. - # On push events the actor isn't a reliable signal (admins can merge - # the bot's PR by hand) -- diff content is the only check. + # on main only touches the allowed-files set with version-shaped + # diffs -- same code, just version-bumped. Re-running the full + # matrix would be pure waste, and release-please.yml's pypi-publish + # builds its own wheel before publishing. Anything else (feature + # merges, direct admin pushes) gets the full matrix. + # On push events the actor isn't a reliable signal (admins can + # merge the bot's PR by hand) -- diff content is the only check. if [ "$EVENT_NAME" = "push" ]; then CHANGED="$(git diff --name-only HEAD~1...HEAD)" echo "Changed files in this push:" echo "$CHANGED" UNEXPECTED="$(is_unexpected "$CHANGED")" - if [ -n "$CHANGED" ] && [ -z "$UNEXPECTED" ]; then - echo "Classification: release-please release commit on main (release-please files only)." + if [ -n "$CHANGED" ] && [ -z "$UNEXPECTED" ] && verify_version_shape HEAD~1; then + echo "Classification: release-please release commit on main (version-bump shape)." echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" echo "should_build=false" >> "$GITHUB_OUTPUT" exit 0 fi echo "Classification: regular push to main." echo "mode=full" >> "$GITHUB_OUTPUT" - echo "should_build=true" >> "$GITHUB_OUTPUT" + echo "should_build=$(compute_should_build HEAD~1)" >> "$GITHUB_OUTPUT" exit 0 fi @@ -107,41 +165,32 @@ jobs: echo "Changed files in this PR:" echo "$CHANGED" - # release-please bypass on PR: requires ALL THREE: + # release-please bypass on PR: requires ALL FOUR: # 1. Branch prefix matches `release-please--` # 2. PR author is the release-please GitHub App # 3. Diff contains only release-please-managed files + # 4. pyproject.toml/__init__.py diffs are version-bump-shaped # Any one missing -> fall through to full CI. if [[ "$HEAD_REF" == release-please--* ]] && [ "$PR_AUTHOR" = "$BOT_AUTHOR" ]; then UNEXPECTED="$(is_unexpected "$CHANGED")" - if [ -z "$UNEXPECTED" ]; then - echo "Classification: release-please bot PR (release-please files only)." + if [ -z "$UNEXPECTED" ] && verify_version_shape "origin/${BASE_REF}"; then + echo "Classification: release-please bot PR (version-bump shape)." echo "mode=release-please-bypass" >> "$GITHUB_OUTPUT" echo "should_build=false" >> "$GITHUB_OUTPUT" exit 0 fi - echo "release-please-- branch from bot contains unexpected files; falling through to full CI." - echo "Unexpected files:" - echo "$UNEXPECTED" + if [ -n "$UNEXPECTED" ]; then + echo "release-please-- branch from bot contains unexpected files; falling through to full CI." + echo "Unexpected files:" + echo "$UNEXPECTED" + fi elif [[ "$HEAD_REF" == release-please--* ]]; then echo "release-please-- branch but author is '$PR_AUTHOR' (not '$BOT_AUTHOR'); running full CI." fi - # Default: full CI. Build only when packaging-relevant files changed - # or a non-Python file under src/ is added or modified (data files - # need explicit package-data inclusion or they're silently dropped - # from the wheel; content edits to existing data files should also - # be exercised by validate-wheel.sh). + # Default: full CI; build conditional on content. echo "mode=full" >> "$GITHUB_OUTPUT" - if echo "$CHANGED" | grep -qE '^(pyproject\.toml|Makefile|MANIFEST\.in|scripts/validate-wheel\.sh)$'; then - echo "should_build=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - if git diff --name-only --diff-filter=AM "origin/${BASE_REF}...HEAD" | grep -E '^src/.+' | grep -qv '\.py$'; then - echo "should_build=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - echo "should_build=false" >> "$GITHUB_OUTPUT" + echo "should_build=$(compute_should_build "origin/${BASE_REF}")" >> "$GITHUB_OUTPUT" # Fast-fail formatting/lint check. Installs only the dev dependency group # from the lockfile (warm-cache install is ~1s), then runs ruff via diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index b008cf9e..199fbd38 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -118,14 +118,15 @@ jobs: passed = (total - failures) if total is not None else None + def _num(v): + return str(v) if v is not None else "-" + def _dur(v): + return f"{v:.1f}s" if v is not None else "-" + print("# E2E Results\n", file=out) print("| Status | Passed | Failed | Total | Duration |", file=out) print("|---|---|---|---|---|", file=out) - print(f"| {status} | " - f"{passed if passed is not None else '-'} | " - f"{failures if failures is not None else '-'} | " - f"{total if total is not None else '-'} | " - f"{duration:.1f}s |" if duration is not None else "- |", + print(f"| {status} | {_num(passed)} | {_num(failures)} | {_num(total)} | {_dur(duration)} |", file=out) print("", file=out)