ci(platform): add on-demand backend CI run (workflow_dispatch) with optional PR coverage refresh - #13434
Conversation
Adds a workflow_dispatch trigger to platform-backend-ci.yml so the full backend lint / type-check / test + coverage suite can be run on demand against any branch, without pushing a new commit: gh workflow run platform-backend-ci.yml --ref <branch> Automatic behavior is unchanged: the existing push / pull_request (with their paths filters) and merge_group triggers are kept exactly as-is. This only adds an on-demand entry point that runs the same test job. Documented under 'Running backend CI on demand' in backend/TESTING.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Sw1B531emT5AyhbWp46Hh
WalkthroughAdds a manual GitHub Actions trigger for backend CI, updates Codecov PR handling for dispatched runs, and documents the manual run and PR coverage refresh flow. ChangesBackend CI on-demand run
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 0 conflict(s), 0 medium risk, 2 low risk (out of 2 PRs with file overlap) Auto-generated on push. Ignores: |
Adds an optional pr_number input to the workflow_dispatch trigger and wires it to the Codecov upload step's override_pr. When the backend suite is run manually against a branch with an open PR (gh workflow run ... -f pr_number=<PR#>), the coverage upload is attached to that PR so Codecov re-evaluates its codecov/project/platform-backend status against the PR base — i.e. a manual run can refresh a stale coverage check on a branch that never triggered backend CI. override_pr resolves to empty for all automatic events (push / pull_request / merge_group), so their normal PR/commit detection is unchanged. Documented in backend/TESTING.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Sw1B531emT5AyhbWp46Hh
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #13434 +/- ##
==========================================
- Coverage 74.71% 74.69% -0.03%
==========================================
Files 2537 2537
Lines 192315 192310 -5
Branches 18925 18925
==========================================
- Hits 143697 143641 -56
- Misses 44493 44535 +42
- Partials 4125 4134 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/review |
There was a problem hiding this comment.
📋 Automated Review — PR #13434
PR #13434 — ci(platform): add on-demand backend CI run (workflow_dispatch) with optional PR coverage refresh
Author: ntindle | Files: 2
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why + What + How — the description explains the path-filter gap it solves, the added workflow_dispatch/pr_number mechanics, and is unusually honest about the caveat that the refresh path can't be live-validated until it reaches master.
What This PR Does
Backend CI (platform-backend-ci.yml) previously ran only on push/pull_request/merge_group, so branches that don't match the paths: filter (frontend/docs-only) could carry stale backend coverage and never refresh a PR's Codecov status. This PR adds a manual workflow_dispatch trigger that runs the full lint/type-check/test suite on demand, plus an optional pr_number input wired into Codecov's override_pr so the coverage upload can be attached to an open PR to refresh its status. Automatic triggers are provably unaffected — the override_pr expression resolves to '' for every non-dispatch event.
Specialist Findings
🛡️ Security ✅ — CI-only change, no app code, endpoints, secrets, or DB objects. Confirmed no shell-injection vector (pr_number is used only in a with: expression, never a run: step) and no secret-exposure regression. One 🟡 check-integrity note: a write-access dispatcher could attach an arbitrary branch's coverage to any PR via override_pr (.github/workflows/platform-backend-ci.yml:415).
🏗️ Architecture ✅ — Clean additive trigger; the test job graph and matrix are untouched. Correct event gating. Flagged one 🟠 durability issue in autogpt_platform/backend/TESTING.md:316 (see below) and noted pr_number has no numeric validation. Also observed a benign concurrency interaction: a dispatch and a push on the same SHA serialize under backend-ci-<sha> rather than run in parallel — not a defect.
⚡ Performance ✅ — No runtime code. Trigger is manually gated, so no automatic/recurring compute added; the override_pr expression is O(1) template evaluation. Same-SHA dispatch de-duplicates against automatic runs via the concurrency key rather than multiplying them.
🧪 Testing ✅ — No runnable test surface for a YAML+docs change. The sole behavioral change (override_pr gating) is verified correct by inspection but is genuinely unverifiable end-to-end until the trigger lands on master — honestly documented, with a required post-merge validation dispatch called out.
📖 Quality ✅ — Readability grade A. YAML comments explain why (path-filter skips, stale carried-forward coverage) not what; TESTING.md is thorough. No smells, dead code, or convention violations.
📦 Product ✅ — Implementation matches the description exactly. 🟡 UX note: a mistyped or mismatched pr_number silently attaches the dispatched HEAD's coverage to the wrong PR with no feedback (.github/workflows/platform-backend-ci.yml:415); docs should state the dispatched --ref must be the PR's head branch.
📬 Discussion gh token → HTTP 401). No blocking issues found in the diff itself. The author's own post-merge codecov-refresh validation remains an open follow-up to track. GitHub CI status is therefore UNVERIFIED.
🔎 QA ✅ — Validated by execution: YAML parses (4 triggers), pr_number input is well-formed, override_pr is wired to the correct codecov/codecov-action@v5 step, and the expression truth table confirms override_pr='' for push/pull_request/merge_group and blank dispatch, and =pr_number only on dispatch-with-input. No runtime surface exists to screenshot; no defects found.
🟠 Should Fix
- Merge-transient validation note becomes misleading after merge (
autogpt_platform/backend/TESTING.md:316) — The note says the refresh path "can't be exercised until this change reachesmaster— validate it with one real dispatch then." That describes this PR's state, not the merged code; once it lands, a future reader is told the path is unvalidated and gated on reachingmasterwhen it already has. Drop the transient sentence and keep only the durable GitHub constraint (workflow_dispatchrequires the trigger on the default branch); move the "validate once on master" step to the PR description or an issue. (Flagged by: architect — 1 specialist, medium severity) - Document that dispatched
--refmust match the PR head (.github/workflows/platform-backend-ci.yml:415) —pr_numberflows straight intooverride_prwith no check that the dispatched branch is that PR's head, so a typo or mismatched dispatch silently attaches coverage from the wrong commit. Add a one-line warning in the inputdescriptionandTESTING.mdthat the--refmust be the PR's head branch. (Flagged by: security, architect, product — 3 specialists)
🟡 Nice to Have
- Numeric validation for
pr_number(.github/workflows/platform-backend-ci.yml:29) — Input is a free-formstring; a small guard rejecting non-numeric values before upload would turn a silent no-op into a clear failure. Optional given the manual, privileged audience. (architect, product)
Human Review Needed
NO — This is an isolated CI-config + docs change with no touch to authentication, credential/secret handling, or trust boundaries between services; correctness is provable by inspection and was validated by the QA specialist.
Risk Assessment
Merge risk: LOW | Rollback: EASY (revert 2 files; no runtime state, no migrations)
CI Status
Local harness: ✅ All 5 checks pass (frontend lint/types/unit/build, backend lint). GitHub CI: UNVERIFIED — the discussion specialist could not fetch live check status (invalid gh token, HTTP 401); confirm real GitHub checks before merge.
UI Testing — Variant Results
✅ local: CI-only change adding a valid workflow_dispatch trigger with a correctly-wired override_pr expression that provably leaves automatic runs unchanged; no runtime surface to break and no defects found.
✅ hosted: CI-only PR adding a workflow_dispatch trigger with a correct override_pr expression; static validation passes and the sole unverifiable path (live PR coverage refresh) is impossible pre-master and already documented.
Brings the branch up to date with dev (dream-runtime #13338/#13391/#13332, #13434, #13591, #13743) and clears the reported PR overlap conflict. No textual conflicts: dev touched none of the five files this PR changes (util/service.py, executor/manager.py, copilot/executor/manager.py, notifications/notifications.py, notifications/test_notifications.py). Semantically compatible too — dev's new Scheduler.cleanup() LaunchDarkly teardown runs before super().cleanup(), which is exactly where this PR's closed-event-loop guard lives, and no new caller of run_and_wait() or start_http_server() was introduced.
…meout (Significant-Gravitas#13780) ### Why / What / How **Why.** Roughly half of all `dev` merge-queue enqueues were ejecting PRs whose own checks were fully green, and the time-to-ejection clustered hard around 17-22 minutes. Observed live on 2026-08-04/05: | PR | enqueued | ejected | elapsed | PR's own checks | actual cause | |---|---|---|---|---|---| | Significant-Gravitas#13434 | 02:55 | 03:12 | ~17 min | all green | real test failures (credit suite) | | Significant-Gravitas#13434 | 04:07 (2nd) | 04:29 | ~22 min | all green | **20m job timeout** | | Significant-Gravitas#13743 | 18:33 | 18:54 | ~21 min | all green | **20m job timeout** | | Significant-Gravitas#13575 | 19:05 | 19:26 | ~21 min | all green | **20m job timeout** | (A fifth ejection, Significant-Gravitas#13764 at 03:21→03:42, was also the 20m timeout.) **What.** The `test` job in `platform-backend-ci.yml` had `timeout-minutes: 20`, which sits *below* the job's real p95 runtime. GitHub reports a `timeout-minutes` kill as conclusion **`cancelled`**, not `failure` — which is why this was invisible when reading the merge-queue runs. `.github/workflows/scripts/check_actions_status.py` treats any conclusion outside `success`/`skipped`/`neutral` as a failure, so a timed-out `test` leg makes **`Check PR Status`** fail, and GitHub ejects the PR from the merge queue. **How.** Raise the cap so it guards against a genuinely hung job instead of acting as a performance budget, and remove the single largest source of setup variance from the job. ### Root-cause evidence The three ~21-22 min ejections are all the same mechanism. GitHub's own annotation on the cancelled job (`check-runs/92414409798/annotations`): > `failure | The job has exceeded the maximum execution time of 20m0s` Every timed-out leg died at **1217-1222s** — exactly the 20m0s cap: | run | merge group | leg | job total | pytest step | |---|---|---|---|---| | 31037832829 | pr-13575 | `test (3.12)` | 1222s | 674s (killed) | | 31035323338 | pr-13743 | `test (3.12)` | 1219s | killed | | 30974258262 | pr-13434 | `test (3.12)` | 1217s | 1049s (killed) | | 30972001003 | pr-13764 | `test (3.11)` | 1218s | 1054s (killed) | These were healthy runs killed mid-suite, not hangs — the pytest step was still actively emitting `PASSED` lines when the runner pulled the plug. Sibling matrix legs in the *same* runs passed comfortably, which is what makes this look like a "flake": - run 31037832829: `test (3.11)` 956s ✅, `test (3.13)` 975s ✅, `test (3.12)` **1222s ❌** - run 30972001003: `test (3.12)` 949s ✅, `test (3.13)` 898s ✅, `test (3.11)` **1218s ❌** Two independent variance sources push a leg over the line: 1. **The suite's own runtime.** ~10.6k tests run **serially** — `pytest-xdist` is not a dependency, and the pytest invocation has no `-n`. Measured across 126 `test` legs: pytest step p50 **787s**, max **1093s**. Two of the four kills had entirely normal setup and were killed purely because pytest itself was still running at 1049s/1054s. 2. **Checkout.** The `test` job is the only job using `fetch-depth: 0` (it needs base-branch refs for the poetry.lock version comparison in "Install Poetry"). On run 31037832829 that checkout took **429s** on the leg that died, versus **27s** and **49s** on the two legs that passed — same commit, same run. Measured `test`-leg duration distribution (126 legs): | event | n | p50 | p90 | max | killed at 20m | |---|---|---|---|---|---| | `pull_request` | 78 | 922s | 978s | 1218s | 2 (2.6%) | | `merge_group` | 27 | 950s | 1056s | 1222s | 2 (7.4%) | | `push` | 21 | 928s | 954s | 976s | 0 | `merge_group` carries the heaviest tail. It is also the most damaging place to fail: `merge_group` has no `paths:` filter (GitHub doesn't support one), so **every** merge group runs the full backend suite even for PRs that cannot touch the backend — Significant-Gravitas#13434 only changed `platform-backend-ci.yml` and `TESTING.md`. ### Before / after The meaningful rate for a `timeout-minutes` change is the share of legs the cap kills, not a test pass rate: | | legs exceeding the cap | per-leg | per enqueue (3-leg matrix) | |---|---|---|---| | **Before** (`20m`) | 4 / 126 | 3.2% | ~9.2%; on `merge_group` legs alone 7.4% → **~20.6%** | | **After** (`35m`) | **0 / 126** | 0% | 0% | No leg in the sample has ever come within 14 minutes of the new cap. The longest *completed* leg observed is 1218s (20.3m); the killed legs were truncated, but extrapolating from their pytest progress they would have landed at roughly 21-25m — still comfortably inside 35m, which retains hang detection while leaving ~40% headroom over the worst realistic run. ### Not fixed here (separate issue) The **Significant-Gravitas#13434 02:55 ejection was a genuinely different failure mode** and is *not* addressed by this PR. `test (3.11)` (job 92194348225) failed with 12 failures + 7 errors, all in the credit suite: - First failure: `credit_concurrency_test.py::test_concurrent_spends_insufficient_balance` — `Expected 5 failures, got 4`. One of 10 concurrent `spend_credits` coroutines raised something that was neither a success nor `InsufficientBalanceError`. - Then `test_race_condition_exact_balance` — `ValueError: User not found with ID: exact-balance-…` for a user that had just been created successfully. - Then everything cascaded: ~8 minutes of `25P02 current transaction is aborted, commands ignored until end of transaction block` across `credit_concurrency_test.py`, `credit_integration_test.py`, `credit_metadata_test.py` and `credit_refund_test.py`. I deliberately have **not** shipped a speculative fix for this. My initial hypothesis (a leaked interactive transaction in the spend path) was **disproven**: `credit.py` opens no Prisma interactive transaction anywhere — `_add_transaction` runs a single autocommit `query_raw` CTE with `SELECT … FOR UPDATE`, so it structurally cannot leave a connection in an aborted state. The real poisoning vector is still open, and reproducing it needs the full stack (Postgres + 3-shard Redis cluster + RabbitMQ + ClamAV + FalkorDB), which I could not stand up in this environment. Fixing it on a guess risks introducing a *new* merge-queue failure mode, which is exactly the problem this PR exists to remove. ### Changes 🏗️ - `.github/workflows/platform-backend-ci.yml`, `test` job: - `timeout-minutes: 20` → **`35`**, with a comment recording the measured runtime distribution so it doesn't get tightened back into the failure zone. - Added **`filter: blob:none`** to the `fetch-depth: 0` checkout. This is a blobless partial clone: every ref stays reachable (so the base-branch `poetry.lock` lookup in "Install Poetry" is unchanged) while the blobs for all other branches are never downloaded. If the lazy fetch ever fails, the existing `; true` fallback already degrades to the HEAD poetry version, so the worst case is benign. No configuration, service, port, secret or env changes. Behaviour of the tests themselves is unchanged. ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [ ] I have tested my changes according to the test plan: - [x] `python3 -c "yaml.safe_load(...)"` parses the workflow; `jobs.test.timeout-minutes == 35` and the checkout `with:` block resolves to `{fetch-depth: 0, filter: blob:none, submodules: true}` - [x] `actionlint` on the changed workflow reports **5** shellcheck findings — byte-identical to the count on `dev`, so no new lint issues are introduced (all 5 are pre-existing, on lines this PR does not touch) - [x] Confirmed `test` is the only job referencing `BASE_REF`, so `fetch-depth: 0` is load-bearing there and nowhere else — it is preserved, only made blobless - [x] All `pre-commit` hooks pass on the commit - [ ] End-to-end confirmation that a `merge_group` run completes inside 35m and that the blobless checkout still resolves `git show "origin/$BASE_BRANCH":./poetry.lock` — this can only be observed on CI, and this PR's own `merge_group` run is the test <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Workflow-only timing and checkout tuning; no application code, secrets, or test behavior changes. > > **Overview** > Raises the backend CI **`test`** job cap from **20m to 35m** and documents why: serial ~10.6k-test runs often exceed 20m, GitHub marks timeouts as **`cancelled`**, and merge-queue **`Check PR Status`** treats that as failure—ejecting otherwise green PRs. > > Adds **`filter: blob:none`** on the existing **`fetch-depth: 0`** checkout so base-branch **`poetry.lock`** resolution for Install Poetry stays the same while avoiding full blob downloads that sometimes stretched checkout to hundreds of seconds on one matrix leg. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2814a3a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --- ### CI verification (this PR's own run 31045429279) All three legs green, and both changes behave as intended: | leg | result | job total | headroom to 35m | checkout | pytest | |---|---|---|---|---|---| | `test (3.11)` | ✅ success | 935s (15.6m) | 19.4m | **9s** | 795s | | `test (3.12)` | ✅ success | 751s (12.5m) | 22.5m | **9s** | 624s | | `test (3.13)` | ✅ success | 925s (15.4m) | 19.6m | **14s** | 773s | **Checkout: 9s / 9s / 14s**, against **27s / 49s / 429s** on the pre-change baseline (run 31037832829) — the 429s outlier that blew the budget is gone. The one real risk in the checkout change was whether a blobless clone could still resolve the base branch's `poetry.lock`. Confirmed from the `Install Poetry` step log: ``` Found Poetry version 2.2.1 in backend/poetry.lock Found Poetry version 2.2.1 in backend/poetry.lock on dev Using Poetry version 2.2.1 ``` The lazy blob fetch resolves correctly and the base-branch comparison is unchanged. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Summary
Adds a manual
workflow_dispatchtrigger to.github/workflows/platform-backend-ci.ymlso the full backend lint / type-check / test + coverage suite can be run on demand against any branch, and — when that branch has an open PR — can refresh that PR's coverage status.(a) What it does
workflow_dispatchtrigger onplatform-backend-ci.yml— runs the sametestjob as the automatic triggers (full matrix + the existing Codecov upload) for the dispatched branch's HEAD commit.pr_numberinput, wired to the Codecov action'soverride_pron the upload step. When provided, the dispatched run's coverage upload is attached to that PR, so Codecov re-evaluates the PR'scodecov/project/platform-backendstatus against the PR base. This lets a manual run refresh a coverage check that's stale only because the branch never triggered backend CI (e.g. a frontend/docs-only branch, where the path filter skips the automatic backend run and Codecov falls back to carried-forward coverage).push/pull_request(with theirpaths:filters) andmerge_grouptriggers are unchanged, andoverride_prresolves to empty for every non-workflow_dispatchevent, so normal PR/commit detection is untouched.(b) Activation caveat
GitHub only makes a
workflow_dispatchworkflow manually triggerable once the trigger exists on the repository's default branch (master) — even when dispatching another branch via--ref. This PR targetsdev, so the trigger stays inert until it propagatesdev → masterthrough the normal cadence. Documented inTESTING.md.(c) Validation note
Because it isn't dispatchable until it reaches
master, thepr_number/override_prrefresh path could not be live-validated in this PR. After this lands onmaster, it should be validated with one real dispatch against an open PR (confirm thecodecov/project/platform-backendcheck refreshes against the PR base). Note:codecov.ymlhasrequire_ci_to_pass: true, so Codecov waits for the dispatched run's CI to finish before updating the status.Files changed
.github/workflows/platform-backend-ci.yml—workflow_dispatchtrigger +pr_numberinput +override_pron the upload step.autogpt_platform/backend/TESTING.md— "Running backend CI on demand" (how to invoke, the PR-refresh path, and the activation/validation caveats).Scope is intentionally minimal — no
codecov.ymlchanges, nothing else touched.🤖 Generated with Claude Code
https://claude.ai/code/session_014Sw1B531emT5AyhbWp46Hh