mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
DottaandClaude Opus 5.5
2e7c66aac9
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Its GitHub Actions CI shares the `ubuntu-latest` runner pool across all workflows > - #19480 added the `PR merge review notification` workflow. It triggers on `workflow_run`, on every completed `check_run`, and on every commit `status` > - One PR push completes about 60 checks, so it queues about 60 notifier jobs. Each job takes a runner > - The Actions queue now holds about 340 queued runs. About 260 of them are this notifier. Other `ubuntu-latest` jobs wait more than one hour (for example, the `authorize` job of Runner Full-Stack E2E image builds) > - This pull request skips events that cannot complete a green PR state before a runner is allocated, and it collapses the remaining events to the newest one per PR head > - The benefit is that this unjams the Actions queue and keeps the advisory review notification ## Linked Issues or Issue Description Refs #19480 **What happened?** The `PR merge review notification` workflow from #19480 schedules one runner job for each `workflow_run`, `check_run`, and `status` event. One PR push makes about 60+ jobs. The repository Actions queue has about 340 queued runs (about 260 from this notifier at the time of writing). Other `ubuntu-latest` jobs wait more than one hour. **What did you expect?** The notifier requests one advisory merge review when PR verification is complete. It must not use a runner for each intermediate check. **Steps to reproduce** 1. Push to any PR. 2. Run `gh run list --workflow pr-merge-review.yml -L 200 --json event,status`. 3. See tens of queued `check_run`, `status`, and `workflow_run` runs for the same push. ## What Changed - `.github/workflows/pr-merge-review.yml`: the job `if:` now schedules the job only for events that can complete a green PR state: - `workflow_run` from `pull_request` or `pull_request_target` with conclusion `success`, `skipped`, or `neutral`. - `check_run` from a non-Actions app (app id is not `15368`) with conclusion `success`, `skipped`, or `neutral`. GitHub Actions check runs are already covered by the `workflow_run` list. - `status` with state `success`. - GitHub cannot filter `check_run` or `status` at the trigger level. A skipped job makes a run record but does not take a runner. - The job has a concurrency group `pr-merge-review-<PR head SHA>` with `cancel-in-progress: true`. The notifier reads the current PR state, so the newest event subsumes older ones. The group is on the job, not the workflow, so a skipped event cannot cancel a scheduled notifier. - The environment, permissions, trusted default-branch checkout, and secret handling do not change. - `.github/scripts/tests/notify-pr-merge-review.test.mjs`: a new test pins the trigger kinds, the job condition, and the job-level concurrency group. - `doc/PR-MERGE-REVIEW-WEBHOOK.md`: describes the event filter and the concurrency group. **Expected runner jobs per PR push** (typical PR: `PR`, `Storybook Visual`, `commitperclip PR Review` workflows; Greptile, Paperclip Review, Socket (2), Superagent (2) external checks; one Snyk status): | | Before | After | | --- | --- | --- | | Eligible events that take a runner | about 60–70 (every check run + 2 statuses + 3 workflow runs) | at most about 10 (3 workflow runs + 6 external checks + 1 status, only when green) | | Jobs that run to completion | about 60–70 | usually 1–4: events that arrive while a job is pending or running cancel it, so a burst collapses to the newest event. In a backed-up queue, all events for one head collapse to one job | ## Verification - `node --test .github/scripts/tests/notify-pr-merge-review.test.mjs`: 22 tests pass. - The new workflow test fails on the old workflow (red) and passes on the new workflow (green). - `actionlint .github/workflows/pr-merge-review.yml`: no findings. - I confirmed with `gh run list` that `workflow_run.head_sha` for the `pull_request_target` workflow (`commitperclip PR Review`) is the PR head SHA. So one SHA key groups all event kinds for one PR head. - I confirmed that all workflows that make PR checks are in the `workflow_run` list (`CI routing` and `Trusted PR CI` are reusable workflows called by `PR`). - After merge: check that `gh run list --workflow pr-merge-review.yml` shows mostly `skipped` runs for each push, and a small number of runs that execute. ## Risks - A check provider that reports a final green state through a GitHub Actions check run from a workflow that is not in the `workflow_run` list does not trigger the notifier. All current PR workflows are in the list. The doc already tells authors to add new PR workflows to the list. - A failed or cancelled check that later succeeds on re-run still triggers the notifier, because the re-run completes with `success`. - `cancel-in-progress` can cancel a notifier during delivery. The newer job evaluates the same or a more complete state, and the Paperclip idempotency key deduplicates deliveries for the same PR, head, and base. - Each event still makes a workflow run record. Skipped runs do not take a runner. - Low risk otherwise: the notifier is advisory and `continue-on-error`. It does not affect PR verification results. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, GitHub CLI). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <[email protected]>