ci: add backend-ci-gate, a Backend CI check that can actually be required

[JSv4]

Why

main has no required_status_checks at all:

$ gh api repos/Open-Source-Legal/OpenContracts/branches/main/protection
{ "required_pull_request_reviews": {...}, "enforce_admins": {...}, ... }   # no required_status_checks key

Nothing gates a merge on CI having run, let alone passed. PR #2262 merged with Backend CI having never run on its head commit:

$ gh api repos/.../commits/fc21249c1/check-runs      # #2262's head
CLAAssistant  success ...  claude-review  success  ...   # no changes / linter / pytest at all

With nothing required, "no check reported" is not a blocker. The push that then merged it (03e37b659) failed at the linter, which skipped pytest — 0 seconds. main sat that way for ~30 hours, repaired only by accident when PR #2266's pre-commit run --all-files happened to reformat the same file.

Why "just require the pytest job" isn't the fix

Requiring something would have blocked #2262 (a check that never reports stays Pending). But requiring pytest specifically leaves a second hole open and opens a third:

1. A job skipped by its own if: reports SUCCESS to branch protection. (GitHub docs: "If a job within a workflow is skipped due to a conditional, it will report its status as Success.") pytest is gated if: needs.linter.result == 'success', so a red linter skips it and the required check still reads green. Not hypothetical — three open PRs are in that state right now:

   PR	head	linter	pytest
   Bump djangorestframework-stubs from 3.17.1 to 3.18.0 #2260	0469df80d	failure	skipped
   Bump mypy from 2.3.0 to 2.3.1 #2264	d27a4d632	failure	skipped
   Bump django-stubs from 6.0.7 to 6.1.0 #2265	5bd1cda74	failure	skipped

   Under a required-pytest policy all three are mergeable today with a red linter.

2. A workflow skipped by path filtering never reports at all, so the required check hangs Pending and blocks forever. With paths-ignore: [docs/**] on the pull_request trigger, requiring any job in this workflow would make docs-only PRs permanently unmergeable.

What this adds

A gate job (check name backend-ci-gate) that always runs and inspects the other jobs' results itself, distinguishing "skipped because this PR touches no backend code" from "skipped because something upstream broke".

• Decision table: .github/scripts/backend_ci_gate.sh, run with --self-test (13 cases, ~0.1s) before each evaluation — a gate whose own logic has silently inverted is worse than no gate.
• paths-ignore dropped from the pull_request trigger only. The changes filter still keeps the expensive jobs off, so a docs-only PR now costs two ubuntu-latest jobs of a few seconds. push keeps its filter.
• .github/scripts/require_backend_ci_gate.sh applies the protection change. It exists because the obvious gh api call is a footgun: PUT .../branches/main/protection replaces the entire object (silently dropping the review rules and the force-push/deletion bans unless re-sent), and the narrower PATCH .../protection/required_status_checks 404s with "Required status checks not enabled" when none exists yet. It also refuses to require a context name that has never been reported on the branch — requiring a typo'd name blocks every PR with no error anywhere.

Verification

Replayed the gate over the last 60 real Backend CI runs:

event	linter	pytest	gate	run conclusion	n
pull_request	failure	skipped	BLOCK	failure	8
pull_request	failure	cancelled	BLOCK	cancelled	1
pull_request	success	failure	BLOCK	failure/cancelled	3
pull_request	skipped	skipped	ALLOW	success	3
pull_request	success	success	ALLOW	success/cancelled	25
push	failure	skipped	BLOCK	failure	2
push	success	success	ALLOW	success	13

No genuinely green run is blocked. Mutation-checked too: neutering the two result comparisons makes the self-test fail 7/13 rather than pass silently. require_backend_ci_gate.sh dry-run verified to reproduce the current protection object byte-for-byte plus the new key, and to refuse backend-ci-gate today (not yet on main).

Follow-up — merging this changes nothing on its own

Three things, in this order:

1. Merge this.
2. Confirm the skip path on the wire. This PR's own diff touches backend.yml, so it only exercises backend=true. Open a one-line docs-only PR and confirm backend-ci-gate reports success with linter/pytest skipped, and read the check name off gh api .../check-runs rather than trusting this description.
3. Then bash .github/scripts/require_backend_ci_gate.sh --apply. The 8 currently-open PRs will each need a merge from main before the check appears on their heads.

Two governance decisions this PR deliberately does not make:

• enforce_admins is false. Admins keep "merge without waiting for requirements to be met", so the gate is advisory for exactly the merge path feat(agents): system_instructions_mode — let an agent config EXTEND the corpus persona instead of replacing it #2262 took. Requiring the gate without flipping this is still a real improvement, but it is not airtight.
• Six other workflows use workflow-level paths: filters, so their checks are not safely requirable as-is — each would hang Pending forever on any PR its filter excludes. Swept rather than spot-checked: frontend.yml, frontend-e2e.yml, frontend-e2e-extract.yml, frontend-e2e-websocket.yml, production-stack.yml and redis-integration.yml. Each needs the same gate treatment before it can join the required list. (backend.yml keeps paths-ignore on push only, which is safe — branch protection does not gate pushes.)

🤖 Generated with Claude Code

[claude]

Reviewed the workflow change and the gate script's decision table against the actual if: conditions in backend.yml (traced changes → linter → pytest → gate for both push and pull_request, including the continue-on-error/fail-open path on changes). The core design is sound and directly addresses the failure mode described (a job skipped by its own if: reporting success, and a workflow skipped by paths-ignore never reporting at all). A few notes:

Process nit (per this repo's CLAUDE.md Baseline Commit Rules #3 — "Never credit Claude or Claude Code in commit messages, PR messages, comments, or any other artifacts")

• The commit message carries Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>.
• The PR description ends with "🤖 Generated with Claude Code".

Both should be stripped before merge per the project's own stated convention.

Logic check (no bugs found, one latent operational caveat)
Walked all reachable (event, backend, linter, pytest) tuples:

• pull_request + backend=false → both jobs skip (matches, since linter's if is push || backend != 'false') → ALLOW, with the defensive failure/cancelled override correctly catching the case where the filter is wrong and a job ran anyway.
• pull_request + backend=true (or '', i.e. the changes job errored and fails open) → strict branch requires both success → correctly BLOCKs the exact feat(agents): system_instructions_mode — let an agent config EXTEND the corpus persona instead of replacing it #2262 shape (linter=failure, pytest=skipped) and any bare pytest=failure.
• push → changes never runs (if: github.event_name == 'pull_request'), so backend='' always, correctly routed to the strict branch regardless of event.

One thing worth being aware of operationally (not a bug in this PR, and not something it needs to fix): gate's needs: [changes, linter, pytest] means it won't start until pytest finishes, is skipped, or is cancelled — needs: isn't bypassed by if: always(), only the run-despite-upstream-failure behavior is. If the self-hosted yuge runner is ever offline, pytest can sit queued for up to its 180-minute timeout-minutes, and once gate is wired into required_status_checks (the follow-up PUT in the description), that means a stuck runner turns into a required check pending for up to 3 hours rather than a merge that just goes through anyway (today's status quo). Might be worth a much shorter timeout-minutes on pytest specifically for the queuing case, or documenting the expected worst case — up to you whether that's worth a follow-up.

Self-test / script

• --self-test running unconditionally before every real evaluation, and the description of mutation-testing the two comparisons, is a good defensive pattern for a script that's effectively a merge gate.
• The set -- $row word-splitting + [ "$backend" = "''" ] && backend="" shim in self_test() is a little fragile-looking but is self-contained test scaffolding, not the production code path (the real script always receives 4 already-quoted args from the workflow) — fine as-is.
• Minimal permissions: contents: read on the new gate job, checkout is required only to pull the script — no security concerns.

Test coverage

• No way to unit-test GitHub Actions if: conditions directly, but the embedded self-test (13 cases covering every state the workflow can produce, including the exact regression shape) plus the 60-run historical replay described in the PR body is a reasonable substitute for infrastructure code like this.

Changelog fragment (changelog.d/backend-ci-required-gate.added.md) follows the repo's fragment convention correctly.

Nice catch overall — the two-sided failure mode (skip-reports-success vs. path-filter-never-reports) is a real and easy-to-miss GitHub Actions gotcha, and the historical replay + self-test give good confidence this won't regress silently.

[claude]

Review

This is unusually well-engineered CI infra — the decision table in backend_ci_gate.sh is self-tested (13 cases covering every reachable (event, backend, linter, pytest) combination given the actual if: conditions in backend.yml), and I traced each case against the real job conditions in the workflow (changes's continue-on-error, linter's always() && (push || backend != 'false'), pytest's added linter.result == 'success' gate) — the self-test rows match what those conditions can actually produce, including the fail-open path when changes errors (backend='', which correctly falls through to the strict success-required branch rather than being mistaken for 'false'). No correctness bugs found in the gate logic itself.

Minor, forward-looking robustness notes on require_backend_ci_gate.sh

Not blockers (this PR only ships the script; --apply is explicitly deferred to a later manual step per the PR description), but worth being aware of since the script's whole stated purpose is avoiding silent field loss on the PUT:

1. required_status_checks.checks fully replaces rather than merges.

   "required_status_checks": {
       "strict": False,
       "checks": [{"context": os.environ["CONTEXT"]}],
   },

   Today main has no required_status_checks object at all, so this is a no-op concern right now. But if this script is ever re-run later (e.g., to add a second required check from another workflow, per the PR's own follow-up note about frontend.yml etc. needing "the same gate treatment"), it will silently drop any checks entries and any strict: true that existed at that point, rather than merging into the existing list. Worth a comment or a merge-in-place before this script gets reused for check Bump traefik from v2.8.7 to v2.9.1 in /compose/production/traefik #2.

2. required_pull_request_reviews round-trip omits dismissal_restrictions and bypass_pull_request_allowances. The rebuilt object only carries dismiss_stale_reviews, require_code_owner_reviews, require_last_push_approval, and required_approving_review_count. If the branch ever has dismissal restrictions or bypass allowances configured (not the case today, per the PR description's listed fields), this script would silently drop them on apply — the same class of bug the script's docstring calls out as the reason PUT is dangerous in the first place.

Neither affects the current main protection state (confirmed by the PR's own dry-run description), so these are just things to keep in mind before this script gets a second use.

Everything else

• backend.yml: dropping paths-ignore from pull_request while keeping it on push is the right call and well-justified (required status checks gate PR merges, not raw pushes) — the docs-only-PR cost (changes + gate, a few seconds) is a reasonable trade for closing the Pending-forever hole.
• The gate job's own trust boundary is worth a one-line mental note even though it's not new: since pull_request (not _target) checks out the PR's own ref, a malicious contributor could in principle edit backend_ci_gate.sh (or backend.yml itself) in their own PR to force ALLOW. That's pre-existing to any repo-defined check (same is already true of the pytest/linter jobs today) and not something this PR needs to solve, just flagging for completeness since the whole PR is about closing gaps in what "green" means.
• Changelog fragment follows the changelog.d/<slug>.<type>.md convention correctly and is appropriately detailed.
• Good use of set -euo pipefail, explicit shellcheck disable=SC2086 with rationale for the intentional word-splitting in the self-test loop, and executable bits set on both new scripts.

Nice work — the "replay over the last 60 real runs" + mutation-testing verification in the PR description is a good bar for this kind of change.

[claude]

Review

This is a well-reasoned fix for a real gap (PR #2262 merging with Backend CI never having run). The decision-table approach in backend_ci_gate.sh, the self-test-before-evaluate ordering, the preflight check in require_backend_ci_gate.sh that refuses to require a context that's never been reported, and the historical replay against 60 real runs are all good practices - this is clearly more careful than the "just require pytest" alternative it explicitly argues against.

A few things worth addressing before (or shortly after) merge, mostly in require_backend_ci_gate.sh since it's the piece that mutates live branch protection:

1. The rebuilt object doesn't actually merge required_status_checks.checks (medium)

.github/scripts/require_backend_ci_gate.sh (~line 87-90):

"required_status_checks": {
    "strict": False,
    "checks": [{"context": os.environ["CONTEXT"]}],
},

This hardcodes the checks list to just the new context rather than reading and appending to cur.get("required_status_checks", {}).get("checks", []). Today it's harmless because main has no required_status_checks at all. But the script's own header comment says the current object is "read back and re-sent with the new key merged in," and the changelog fragment claims it "applies the branch protection without clobbering the rest of the object" - neither is true for this field specifically. If this script is ever re-run after a second required check exists (e.g. a future frontend-ci-gate, or one added manually via the UI), it will silently un-require it. Worth either merging the existing checks array in, or adding a loud comment noting this script is only safe to run when backend-ci-gate is the only intended required check.

2. required_pull_request_reviews drops dismissal_restrictions and bypass_pull_request_allowances (medium)

Same file, the rpr rebuild only copies four fields:

out["required_pull_request_reviews"] = {
    "dismiss_stale_reviews": rpr.get("dismiss_stale_reviews", False),
    "require_code_owner_reviews": rpr.get("require_code_owner_reviews", False),
    "require_last_push_approval": rpr.get("require_last_push_approval", False),
    "required_approving_review_count": rpr.get("required_approving_review_count", 0),
}

If main ever has dismissal restrictions or bypass allowances configured, this round-trip silently clears them - same "not actually a full merge" gap as #1, and same caveat that it's not exercised by today's main state.

3. strict is hardcoded to False rather than preserved (minor / by design)

This one is explicitly reasoned about in the inline comment (avoid forcing rebase-before-merge as an unrelated behavior change), so it's a deliberate choice rather than an oversight - flagging mainly because it's another place where "read back and merged in" isn't quite what happens. If main's required_status_checks.strict is ever intentionally set to true later, a re-run of this script would flip it back to false with no warning.

4. Missing app_id on the required check (nit)

The check entry is {"context": "backend-ci-gate"} with no app_id, so in principle any integration (not just this GitHub Actions workflow) could satisfy the required context by reporting a status with that name. Probably fine given the current threat model, but worth a deliberate call-out if status spoofing across apps/bots is a concern for this repo.

5. changes job's path filter doesn't cover .github/scripts/** (worth noting, lower severity than it first looks)

The backend filter in .github/workflows/backend.yml lists opencontractserver/**, config/**, .github/workflows/backend.yml, etc., but not .github/scripts/**. A PR touching only backend_ci_gate.sh/require_backend_ci_gate.sh gets backend=false, which skips linter/pytest. However, the gate job's "Self-test the gate logic" step runs unconditionally (if: always() on the job itself, not gated by the filter), so a change that breaks one of the 13 fixed self-test cases is still caught. The real residual gap is narrower than "zero coverage": a subtle evaluate() bug that happens not to flip any of the 13 existing cases would ship with a green gate and no lint/mypy pass over the changed script (there's no shellcheck hook in .pre-commit-config.yaml, so linter coverage for .sh files is minimal even when linter does run). Might be worth adding .github/scripts/** to the filter anyway, mostly so pytest's green check is meaningful evidence the repo still builds/tests after a workflow-adjacent change.

Non-blocking observations

• Good call documenting the two follow-up steps (confirm the skip path on a docs-only PR, then run --apply) - this correctly isn't a "merge and done" change, and the PR description is upfront about that.
• enforce_admins: false is called out explicitly as a known gap (admins can still bypass) rather than silently left out - appreciated.
• The six other workflows with unsafe paths: filters are listed rather than glossed over. Consider filing a tracking issue for those follow-ups so they don't get lost.

Overall: solid design for the actual CI gate logic; the main thing I'd want resolved is tightening require_backend_ci_gate.sh so its "byte-for-byte / non-clobbering" claims hold for the fields it currently drops (checks, dismissal_restrictions, bypass_pull_request_allowances), even if none of those are populated on main today.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!