Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 67 additions & 6 deletions .github/workflows/provider-verifier.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,18 @@ name: Provider Verifier Gate
on:
pull_request:
branches: [main]
paths:
- 'Pmoves-MiniMax-Provider-Verifier/**'
- 'pmoves/tools/provider_verifier_gate.py'
- 'pmoves/tools/tests/test_provider_verifier_gate.py'
- '.github/workflows/provider-verifier.yml'
# NO paths: filter, deliberately -- see the `changed` step below.
Comment thread
POWERFULMOVES marked this conversation as resolved.
#
# GitHub: "If a workflow is skipped due to path filtering, branch filtering
# or a commit message, then checks associated with that workflow will remain
# in a Pending state. A pull request that requires those checks to be
# successful will be blocked from merging." and "You should not use path or
# branch filtering to skip workflow runs if the workflow is required."
#
# A skipped WORKFLOW never reports. A skipped JOB reports success. Since this
# gate is meant to become a required check, the path decision has to move
# from the trigger into the job, or every PR that touches none of these paths
# would sit on "Waiting for status to be reported" forever.
workflow_dispatch:
inputs:
providers_json:
Expand Down Expand Up @@ -61,19 +68,72 @@ jobs:
# comment step post on FAIL.
permissions:
issues: write
# the `changed` step lists PR files via the API
pull-requests: read
steps:
- name: Checkout repo (with submodules)
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
submodules: recursive
fetch-depth: 1

# Decide applicability HERE rather than in on.pull_request.paths. The job
# always runs and always reports; the work below is conditional. That is the
# shape GitHub prescribes for a workflow that is a required check.
# Decide applicability HERE rather than in on.pull_request.paths. The job
# always runs and always reports; the work below is conditional. That is the
# shape GitHub prescribes for a workflow that is a required check.
#
# Uses the PR files API, not a local diff: checkout is fetch-depth 1, so
# neither `git diff base..head` nor `HEAD~1` resolves -- the first attempt
# here died with "ambiguous argument 'HEAD~1'". Deepening the clone would
# mean a full history fetch of a monorepo with 72 submodules to answer a
# question the API answers in one call.
#
# FAILS OPEN TOWARD RUNNING: if the API call fails we set relevant=true, so
# an unanswerable question runs the gate rather than silently skipping it.
- name: Does this PR touch the verifier surface?
id: changed
shell: bash
env:
GH_TOKEN: ${{ github.token }}
run: |
set -uo pipefail
num="${{ github.event.pull_request.number }}"
if [ -z "$num" ]; then
echo "no PR context (workflow_dispatch) — running the gate."
echo "relevant=true" >> "$GITHUB_OUTPUT"; exit 0
fi
files="$(gh api --paginate "repos/${{ github.repository }}/pulls/$num/files" --jq '.[].filename' 2>/dev/null)"
if [ -z "$files" ]; then
echo "::warning::could not list PR files — running the gate rather than skipping it"
echo "relevant=true" >> "$GITHUB_OUTPUT"; exit 0
fi
relevant=false
while IFS= read -r f; do
[ -n "$f" ] || continue
case "$f" in
Pmoves-MiniMax-Provider-Verifier/*) relevant=true ;;
pmoves/tools/provider_verifier_gate.py) relevant=true ;;
pmoves/tools/tests/test_provider_verifier_gate.py) relevant=true ;;
.github/workflows/provider-verifier.yml) relevant=true ;;
esac
done <<< "$files"
echo "relevant=$relevant" >> "$GITHUB_OUTPUT"
if [ "$relevant" = "true" ]; then
echo "Verifier surface touched — running the static gate."
else
echo "No verifier-surface change; gate is a no-op and reports success."
fi

- name: Set up Python
if: steps.changed.outputs.relevant == 'true'
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
with:
python-version: '3.12'

- name: Install the verifier's runtime deps (best-effort)
if: steps.changed.outputs.relevant == 'true'
# The gate's 6 static checks don't need the verifier's heavy
# deps (numpy, openai, megfile). We install them only for the
# full-conformance dispatch path. The 'best-effort' install
Expand All @@ -85,6 +145,7 @@ jobs:
echo "::warning::verifier runtime deps not installed; the static gate still runs"

- name: Run the static gate
if: steps.changed.outputs.relevant == 'true'
id: gate
shell: bash
run: |
Expand All @@ -110,7 +171,7 @@ jobs:
echo "verdict=PASS" >> "$GITHUB_OUTPUT"

- name: Post PR comment on FAIL
if: failure()
if: failure() && steps.changed.outputs.relevant == 'true'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
Expand Down
56 changes: 41 additions & 15 deletions pmoves/tests/test_provider_verifier_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,26 +77,52 @@ def test_workflow_triggers_on_pull_request(workflow: dict) -> None:
)


def test_workflow_paths_filter_covers_submodule(workflow: dict) -> None:
"""The paths filter must include Pmoves-MiniMax-Provider-Verifier/**.

This is the load-bearing path filter: a PR that adds a new provider
to the cascade touches the submodule, and the gate must run.
def test_workflow_has_no_paths_filter(workflow: dict) -> None:
"""on.pull_request must NOT carry a paths: filter.

This gate is intended to become a required status check, and GitHub is
explicit: "If a workflow is skipped due to path filtering ... checks
associated with that workflow will remain in a Pending state. A pull request
that requires those checks to be successful will be blocked from merging."
A skipped WORKFLOW never reports; a skipped JOB reports success. So the path
decision belongs in the job, not the trigger -- see the two tests below,
which assert the same coverage in its new home.
"""
pr = _triggers(workflow)["pull_request"]
paths = pr.get("paths", [])
assert any("Pmoves-MiniMax-Provider-Verifier" in p for p in paths), (
f"pull_request.paths must include a Pmoves-MiniMax-Provider-Verifier "
f"entry; got {paths}"
assert "paths" not in (pr or {}), (
"on.pull_request must not filter by paths: a required check that never "
"reports blocks every PR that touches none of those paths. Move the "
"condition into the job (steps.changed) instead."
)


def test_workflow_paths_filter_covers_helper(workflow: dict) -> None:
"""The paths filter must include pmoves/tools/provider_verifier_gate.py."""
pr = _triggers(workflow)["pull_request"]
paths = pr.get("paths", [])
assert any("provider_verifier_gate" in p for p in paths), (
f"pull_request.paths must include the helper; got {paths}"
def test_job_condition_covers_submodule(workflow_text: str) -> None:
"""The job-level path condition must still cover the verifier submodule.

Same guarantee the old paths: filter carried -- a PR adding a provider to the
cascade touches the submodule and must run the gate -- asserted where the
decision now lives.
"""
assert "Pmoves-MiniMax-Provider-Verifier/*)" in workflow_text, (
"the `changed` step's case must match Pmoves-MiniMax-Provider-Verifier/*"
)


def test_job_condition_covers_helper(workflow_text: str) -> None:
"""The job-level path condition must still cover the gate helper."""
assert "pmoves/tools/provider_verifier_gate.py)" in workflow_text, (
"the `changed` step's case must match pmoves/tools/provider_verifier_gate.py"
)


def test_working_steps_are_gated_on_the_condition(workflow_text: str) -> None:
"""The work must be conditional even though the job is not.

If the steps ran unconditionally the gate would execute on every PR in the
repo; if the JOB were conditional it would stop reporting. Both halves matter.
"""
assert "steps.changed.outputs.relevant == 'true'" in workflow_text, (
"the gate steps must be guarded by the computed condition"
)


Expand Down
Loading