Skip to content

ci: let a pull request produce the "Run tests" check it must pass - #111

Open
accesswatch wants to merge 1 commit into
mainfrom
ci/unblock-pr-checks
Open

ci: let a pull request produce the "Run tests" check it must pass#111
accesswatch wants to merge 1 commit into
mainfrom
ci/unblock-pr-checks

Conversation

@accesswatch

Copy link
Copy Markdown
Collaborator

Why

Branch protection requires a status check named Run tests. That job existed only in deploy.yml, which triggers on push to main and workflow_dispatchnever on pull_request.

So a PR could never produce the check it was required to pass. Every PR blocked indefinitely, enforce_admins was switched off, and pushing directly to main became the only way to land anything.

What this changes

  • .github/workflows/tests.yml (new) — produces Run tests on pull_request and on pushes to main. Deliberately no paths: filter: a path-filtered required check recreates the same deadlock for any PR touching other paths.
  • deploy.yml — its gating job is renamed Pre-deploy tests so only one workflow exports the required context. The deploy is still gated.
  • docs/deployment.md — records that production deploys are automatic on push to main, and that running deploy-app.sh by hand can collide with the Actions deploy. The concurrency: deploy-production group serialises Actions runs against each other but cannot see a person on SSH; two concurrent runs fail in docker compose up with a container-name conflict. This happened during this work.
  • BRANCH_PROTECTION_RULES.md — documents where the check lives and the path-filter trap.
  • web/e2e/tests/regression.spec.mjs — renames the site-audit test to say what it verifies. The SSRF guard refuses private addresses, so a scan aimed at the test server is always refused: the test exercised submission, the results page and artifact links, but never scanning, while its name claimed otherwise. It now also asserts the refusal is reported honestly (failed=1, scanned=0, per-page result error).

Verification

This PR is its own test: if Run tests reports here, the deadlock is fixed.

Once it does, enforce_admins should be turned on so the setting matches the documented policy.

🤖 Generated with Claude Code

Branch protection required a status check named "Run tests", but that job
existed only in deploy.yml, which triggers on push to main and
workflow_dispatch -- never on pull_request. A PR could therefore never
produce the check it was required to pass, so every PR blocked forever,
admin enforcement was switched off, and pushing straight to main became the
only way to land anything.

Moves the check into its own tests.yml running on pull_request and on
pushes to main, deliberately with no paths: filter -- a path-filtered
required check recreates the same deadlock for any PR touching other paths.
deploy.yml keeps its own gating job, renamed "Pre-deploy tests" so only one
workflow exports the required context.

Also documents that production deploys are automatic on push to main, and
that running deploy-app.sh by hand can collide with the Actions deploy: the
concurrency group serialises Actions runs against each other but cannot see
a person on SSH, and two concurrent runs fail in docker compose up with a
container name conflict. That happened during this work.

Renames the site-audit e2e regression test to say what it verifies. The
SSRF guard refuses private addresses, so a scan aimed at the test server is
always refused -- the test exercised submission, the results page and the
artifact links, but never scanning, while its name claimed otherwise. It
now also asserts the refusal is reported honestly (failed=1, scanned=0,
per-page result "error") rather than passing identically whether the run
reported an error or wrongly claimed a clean page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NqHuFZ4bm7SKB4ebBczYgo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant