Repository navigation
ci: route a pull request from its last green head when it only merged main - #14659
Conversation
… main RFC #14631, slice 2. When a pull request's new head is its previous green head plus merges of main (and at most one resolution commit on top), its own changes already passed there. The changes job now diffs from that green head instead of from main, so only what changed since green is routed. scripts/ci/delta_since_green.py walks the head's first-parent chain (commit-only fetch), reads the ci-status verdicts of the candidates in one GraphQL call, checks each merge brought in a main commit, and refuses when a file the pull request now changes was neither in its diff at the green head nor changed since. Anything else fails open to the usual diff. ci.yml runs the base revision's copy, writes the chosen base and reason to the step summary, and the repository variable CI_DELTA_SINCE_GREEN=0 turns it off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
This comment has been minimized.
This comment has been minimized.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 7 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe CI workflow can select a validated green pull request head as the change detector’s diff base. The selector checks Git history, changed paths, and pull request CI verdicts. If it cannot select a base, the workflow uses the usual pull request diff. ChangesCI diff base selection
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant ChangesJob
participant DeltaScript as delta_since_green.py
participant Git
participant GitHubAPI as GitHub API
participant Detector
ChangesJob->>DeltaScript: invoke with merge SHA and pull request details
DeltaScript->>Git: inspect history and changed files
DeltaScript->>GitHubAPI: retrieve pull request CI verdicts
GitHubAPI-->>DeltaScript: return workflow run and check conclusions
DeltaScript-->>ChangesJob: write selected or empty base SHA
ChangesJob->>Detector: pass delta base for change detection
Merge Risk: ⚪ Minimal · up to The selected diff covers changes since the validated green head. No actionable merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The narrower CI route has safeguards and falls back to the usual diff on many failures. Two boundaries still merit review: verdict lookups can omit results beyond an API page, and the new check-reading permission is available to other code run by the job. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
…e full diff Review of #14659: - A pull request whose own diff touches CI policy (.github/, scripts/ci/, tests/test_ci_*, the test registry) now keeps its whole diff. The delta would otherwise drop the router, stdlib-shadow and guard-test inputs that ci.yml classifies by their presence in the diff. - The green head's verdict counts only ci.yml runs (matched by file path) that the runs API ties to this pull request number and base branch; a stacked pull request on another base or a fork run says nothing. - Within one run the latest attempt wins; a red ci-status from any other run of this pull request makes the head red. - The step runs for same-repository pull requests only, where the CI_DELTA_SINCE_GREEN switch can reach it. The workflow test now evaluates the step's condition for forks, other events and the switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Re-review of #14659: the runs API fills a run's pull_requests[].base.ref in at query time, so after a retarget (feature-x to main) the green head's old run would read as a run on main. The verdict query now asks for the pull request's BASE_REF_CHANGED_EVENT timeline items and skips the delta on any, or when the field cannot be read. It reads filteredCount and nodes: totalCount counts the whole timeline whatever itemTypes says (7 on this pull request, which was never retargeted). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#14659's guard run failed on the Linux runners (git 2.52): the --deepen fetch exited 128 in the two-merge fixtures. The chain fetch already completes a short history, and --deepen on a complete repository is what that git refuses. The selector now deepens only while the checkout is still shallow, retries any filtered fetch without its filter, and puts the fallback (with git's stderr) in the step summary. A fetch that fails both ways skips the delta with git's stderr in the reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
|
Merge receipt for
|
…#14987) Delta CI (#14659) routes diff(last green head, merge), which carries everything main gained since the pull request last synced. When main moved further than the pull request, that routes more than the pull request's own diff: #14961 routed 40 files for a 15-file diff and #14960 43 for 10, picked about 130 unrelated unit test classes, and failed suite-coverage on main's cmuxUITests/ edits from #14966. Take the delta only when it is no larger than the pull request diff and carries no cmuxUITests/ edit the pull request did not make. Otherwise route the usual pull request diff, as every other skip does. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Slice 2 of #14631 (delta CI). Slice 1 (catch-up job) is separate.
What changes
When a pull request's head H2 is its previous head H1 plus one or more merges of main, and at most one resolution commit on top, and H1's
ci-statuspassed, thechangesjob diffs from H1 instead of from main. Routing then covers only what changed since green: main's delta and the resolution.scripts/ci/delta_since_green.pyfinds H1:--filter=tree:0).ci-statusfor the candidate commits in one GraphQL call with the workflow token. Onlypull_requestruns of.github/workflows/ci.ymlcount, matched by file path, not by name.pull_requestsinclude this PR number with this base branch. A stacked PR on another base, or a run with no PR attached, says nothing.ci-statusfrom any other run of this PR makes the head red..github/,scripts/ci/,tests/test_ci_*ortests/test-execution.toml, the delta is skipped. Otherwise the trusted-router switch, the stdlib-shadow guard and guard-test self-selection, which ci.yml decides by whether those files are in the diff, could lose them. Main's own policy edits still arrive in the delta and are classified as usual.merge-base(H1, main)..H1). A merge that kept the pull request's side of a file only main had edited fails open.pull_requestevent, a plain new commit, a new commit on a green merge, H1 red or without a verdict, a force push, a merge of a non-main branch, history too shallow (it deepens 3000 commits, commits only), a PR that edits CI policy, or an API error.ci.yml:deltastep runs the base revision's copy of the selector, like the trusted router.delta since green head abc1234567: 3 files (pull request diff: 12 files).BASE_SHA=$DELTA_BASE_SHAonly when one was found. The changes job gainschecks: read.CI_DELTA_SINCE_GREENto0. The step runs for same-repository pull requests only: fork runs get no repository variables, so the switch could not reach them, and GitHub ties no PR to a fork run.Because the step runs the base copy, this PR's own run does not exercise it. The first PR run after merge will.
Tests
tests/test_ci_delta_since_green.py(19 tests). Each builds a fixture origin repo and runs the selector from a depth-2 clone of GitHub's merge commit, as actions/checkout leaves it. Cases:main(), and the ci.yml wiringtests/test_ci_change_areas.py,test_ci_linux_guard_routing.py,test_ci_guard_workflow_structure.py,test_ci_workflow_guards_are_wired.py,test_ci_repo_variable_defaults.py,test_ci_fork_runner_routing.py,test_ci_cli_product_routing.py,test_ci_workflow_path_filter_parity.pyandtest_ci_test_execution_registry.py.actionlintis clean.Known gaps
ci-macos.ymlpicks Swift packages from its ownHEAD^1..HEADdiff on the Mac runner, not from the delta. When the delta routes the package lane, the lane still selects packages from the whole PR diff. That runs more than needed, never less; passing the delta list to that job is a follow-up.diff(H1, merge commit), so if main moved again after H2, that movement is routed too (conservative).🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Routes CI diffing for a pull request whose new head only merged main: instead of diffing against main, the
changesjob diffs from the PR's last green head, so only main's delta plus any resolution commit are tested again.scripts/ci/delta_since_green.pypicks the green head from the head's first-parent chain usingci-statusverdicts, applying only when every commit since is a merge of main (plus at most one resolution commit); everything else fails open to the usual diff..github/,scripts/ci/,tests/test_ci_*, the test registry) keeps its whole diff and never routes from a green head.deltastep inci.ymlruns the base revision's copy and logs the chosen base; set theCI_DELTA_SINCE_GREENrepository variable to0to disable. It defaults to on, and runs for same-repository PRs only.ci-macos.ymlover-selects packages on routed lanes but never under-selects.Written for commit 938fac2. Summary will update on new commits.
Summary by CodeRabbit