Repository navigation
ci(pr-gate): wait for slow Backend CI instead of timing out at 40 min - #3595
Conversation
Signed-off-by: Christopher Sellers <1@christophersellers.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
|
PR Blocker ChecklistHead SHA: Status: Ready
|
|
Vercel preview access note removed (A-2177 secret-hygiene cleanup). |
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: PASS deepseek 040e914
Reviewed the change to .github/workflows/pr-gate.yml (backend job only).
- Run lookup.
listWorkflowRunsfilters onworkflow_id: 'backend-ci.yml',head_sha: HEAD_SHA(the PR head, passed via env) andevent: 'pull_request', so it finds only this commit's PR runs. Schedule and push runs are excluded. - No false pass. The only success path is still the existing
checks.listForRef(check_name="Backend CI Status Gate", filter: latest)withconclusion === 'success'. Every new branch either callssetFailedor keeps polling.- Multiple runs or reruns: the check fails only when every run is
completedand no gate check exists, and only on two consecutive polls (thecompletedWithoutGatereset is in the else branch). - A rerun goes back to
queued, which resets that flag, so the job keeps waiting.
- Multiple runs or reruns: the check fails only when every run is
- 10-minute check. It fires only when
workflowRuns.length === 0. A queued or waiting run is already in the list, so runner contention can't trip it. It catches only real path-list drift, and the message says so. - Permissions. Unchanged:
contents/checks/actions: read, and the top level ispermissions: {}.actions: readis whatlistWorkflowRunsneeds. - Injection. There are no new
${{ }}expressions insidescript:. The interpolated values areCHECK_NAME(a constant) and the API-enumconclusionandstatus. - Timeouts. The 140-minute poll is below the 150-minute job timeout.
callWithRetrysleeps are capped at the deadline.
Non-blocking:
- The pre-existing
runs.find(r => r.status === 'completed')takes the first completed gate run. Withfilter: latestthat's normally the only one, butruns.findon success with a sort bycompleted_atwould be more robust. per_page: 20is fine in practice.
CI: everything is green except Engine Review Pass, PR Blocker Checklist and Head SHA Check Guard, which wait on this review and so are circular. PR Gate: Backend passed.
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: PASS deepseek f2e9e9a
Delta review, by hand. Merge of main 91737af into 040e914 (already PASSed). The remerge-diff is empty. The diff vs main is still only .github/workflows/pr-gate.yml (+37/-4), identical to 040e914.
CI: all real checks green. Remaining failures are engine-review gates only (circular), plus Vercel Preview Built (deploy skipped by Ignored Build Step on non-frontend PRs, not a build failure): Engine Review Pass,Engine Review Pass,visual-report (non-required),PR Blocker Checklist,Head SHA Check Guard,
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: PASS deepseek f2e9e9a
This follows up my earlier hand review. A dsh headless run on the full PR diff (merge-base..f2e9e9a) confirmed the PASS. Excerpt:
- The state machine cannot false-pass: `completedWithoutGate` requires two consecutive completed-without-gate observations, and any newly appearing run resets it.
- I compared the gate's 18 backend paths with `backend-ci.yml`'s 18 `pull_request` paths: identical sets and order, so the new 10-minute "not triggered" failure should only fire on real drift/cancellation, not on ordinary backend PRs.
- The JS block is syntactically valid when wrapped as the async function body `actions/github-script` uses (`node --check` passes).
- No production code, copy, migrations, tenant-scoped queries, or fixtures are touched; the existing workflow-contract checks (`scripts/check_workflow_inventory.py`, `tests/scripts/test_*`) do not pin the old 40/45-minute values.
No concrete bug found in the diff. Non-blocking observations: the new branch logic has no automated test (the repo has no harness for the embedded script), and the 10-minute heuristic could turn a rare GitHub-side run-creation delay into a failure — but it fails closed and is recoverable by rerun, which matches the PR's stated intent.
VERDICT: PASS
Root cause
"PR Gate: Backend" is not hanging on PRs without backend changes: path detection (three-dot diff mirroring backend-ci.yml
paths:) already skips correctly. Every affected PR touched backend paths (e.g. #3586 changesscripts/build_audit_bundle.shandservices/ingestion/tests/…), so Backend CI did trigger.The real failure: the wait step polled "Backend CI Status Gate" for a fixed 40 min (job timeout 45). Under runner contention, Backend Services CI/CD now takes 60–80+ min (jobs sit queued), so the gate times out and fails, and it never re-evaluates when the backend gate later goes green.
Fix (pr-gate.yml backend job only)
timeout-minutes45 → 150; poll deadline 40 → 140 min (still bounded, can't hang forever).success. Frontend job, detection, and the no-backend-change skip are unchanged.Affected PRs (same cause)
After merge, these PRs need main merged in (or a gate rerun) to go green.
Verified locally: YAML parses,
scripts/check_workflow_inventory.pypasses,node --checkpasses on the script. Live Actions behavior has not been tested yet.Note
Medium Risk
Changes merge-blocking PR gate behavior and failure modes for backend PRs; incorrect new logic could block or delay merges, but pass still requires a successful Backend CI Status Gate.
Overview
Fixes PR Gate: Backend failing around 40 minutes while Backend CI is still queued or running (often 60–80+ minutes under runner contention).
The backend job timeout rises from 45 → 150 minutes and the Backend CI Status Gate poll window from 40 → 140 minutes. While waiting, the step now inspects
backend-ci.ymlworkflow runs for the PR head SHA: if backend paths changed but no run appears within 10 minutes, it fails (likely path-list drift vsbackend-ci.yml); if the workflow finishes without a gate check, it rechecks once then fails with the run conclusion; otherwise it keeps polling until a completed gate withsuccess.Frontend gate timing, path detection, and vacuous pass when backend paths are untouched are unchanged.
Reviewed by Cursor Bugbot for commit 040e914. Bugbot is set up for automated code reviews on this repo. Configure here.