fix(scheduler): resolve live refs before cancelling runs - #1348
Conversation
|
Warning Review limit reachedNext included review available in 20 minutes. View limit detailsLimit details: You’ve used the included review 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: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough조직 큐 스윕이 열린 PR의 head SHA를 live Git ref에서 다시 조회합니다. ref 조회 실패나 잘못된 SHA가 있으면 실행 취소를 중단하고 Changes실시간 PR 헤드 검증
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to Queue cleanup can still cancel valid active runs when an open PR has a missing or malformed head reference because that PR is silently omitted from live-head resolution. Fix the fail-closed handling before merging. Sequence Diagram(s)sequenceDiagram
participant QueueSweep as 조직 큐 스윕
participant PullRequestsAPI as GitHub Pull Requests API
participant GitRefAPI as GitHub Git ref API
participant ActionsAPI as GitHub Actions API
QueueSweep->>PullRequestsAPI: 열린 PR의 head 저장소와 브랜치 조회
QueueSweep->>GitRefAPI: head 브랜치의 live Git ref 조회
GitRefAPI-->>QueueSweep: 현재 SHA 반환 또는 조회 실패
QueueSweep->>ActionsAPI: 유효한 이전 head 실행 취소
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (4 skipped: 4 unsupported.) ✨ 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Fresh current-head cancellation canary after a normal non-force ref update: PR #1158 moved to exact head Please use #1158@ |
Fix the empty-open-PR inventory diagnostic without weakening live-ref cancellation evidence.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
# Conflicts: # CHANGELOG.md
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Renumber this branch's new Gap G-15 entry to G-16 to avoid colliding with main's existing G-15 (attachment processing), which merged independently while this branch was open.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
* docs(gaps): restore section dropped by #1423's merge, correct stale claim PR #1423 merged (ebd1b8b) using a merge-conflict resolution that kept only its own branch's dated section and silently dropped main's already-published '2026-08-30 sidecar pin staleness recurrence' section (the #1422 fix's own record) from the file entirely. That merge also left #1423's own section claiming #1413/#1422 'have not merged' even though both are its own git ancestors. Restore the dropped section verbatim, annotate the stale section as superseded, and append an accurate dated entry recording this cycle's backlog-refresh work: 15 PRs merged/retriggered against the new main, 8 left untouched for real conflicts, one independent sidecar/review- dispatch defect newly observed on #1420, and a Gap ID collision (G-15) renumbered to G-16 on #1348. Verified: git diff against origin/main shows this change is purely additive (no main content removed); pytest tests/test_product_technical_gap_baseline.py tests/test_pr_governance_audit_contract.py (8 passed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw * docs(gaps): fix Devin-flagged count/reference errors in #1424 - Clean-merge subtotal said 5 but named 6 PRs; corrected to 6, and added the omitted #1391 to the trivial-conflicts list so 6+7+2=15 reconciles with the stated total. - Compact cross-repo references to the owner/repo#num binding convention: ContextualWisdomLab/contextual-orchestrator#857 and ContextualWisdomLab/LineageWeave#667. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
…e-live-ref-race # Conflicts: # CHANGELOG.md
…e-ref-race # Conflicts: # CHANGELOG.md
|
Resolved the stale merge conflict against current Conflict scope: only
Test evidence on the merged tree:
No product/design decision was in question here — this was a mechanical changelog merge plus disjoint-region auto-merges. The branch is now conflict-free against Generated by Claude Code |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…llision Devin correctly flagged that G-16 was already allocated in this same document (line ~633) to #1348's own queue-hygiene gap, which is still open and unmerged, so reusing it here would collide once #1348 lands. Renumbered the three 2026-09-01-directive gaps to G-19/G-20/G-21 (verified free via a PR body search) and fixed the timeout-floor cross-reference to point at the owner-directive section rather than a Gap ID that does not itself define the 3-hour floor. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 Final cancellation check never runs
revalidate_queue_cancellation.sh has no production caller after this workflow is removed. A synchronized PR can therefore lose its current-head checks to stale cancellation.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
github-project-automation
Bot
moved this from In Progress
to Done
in naruon Platform Roadmap
|
|
||
| case "$event" in | ||
| pull_request|pull_request_target) | ||
| pr_number="$(jq -r '.pull_requests[0].number // empty' <<<"$run_json")" |
There was a problem hiding this comment.
🔴 Secondary pull requests lose current checks
For runs associated with several pull requests, pull_requests[0] validates only the first. Another open pull request can lose its current-head check.
Prompt for agents
Update scripts/ci/revalidate_queue_cancellation.sh to evaluate every pull request listed in the live workflow-run metadata, not only pull_requests[0]. Before cancellation, preserve the run if any associated open PR resolves to the run head. Fail closed if any relevant PR or head ref cannot be read or validated. Add regression coverage for a run associated with multiple PRs where the first is closed or superseded and a later open PR has the run's authoritative current head.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if ! gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel" >/dev/null; then | ||
| echo "Could not cancel ${cancellation_mode} run ${run_id} in ${repo_full_name}; it may have started or finished already." | ||
| fi |
There was a problem hiding this comment.
📝 Info: Cancellation remains best effort
A failed cancellation returns success after logging. The next hourly sweep can retry the still-active candidate without failing unrelated repository processing.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if [ "$live_head_sha" != "$classified_sha" ] || [ "$final_ref_sha" != "$classified_sha" ]; then | ||
| warn_preserve "PR ${pr_number} moved after queue classification." | ||
| fi |
There was a problem hiding this comment.
📝 Info: Head movement defers cleanup safely
Any post-classification head mismatch preserves the candidate. A genuinely obsolete run can survive one sweep, avoiding cancellation from inconsistent intermediate evidence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if ! fresh_open_pr_refs_json="$( | ||
| gh api \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| "/repos/${repo_full_name}/pulls?state=open&per_page=100" \ | ||
| --paginate \ | ||
| | jq -sc '[.[] | .[] | { | ||
| repo: (.head.repo.full_name // null), | ||
| ref: (.head.ref // null) | ||
| }]' | ||
| )"; then | ||
| warn_preserve "open PR heads could not be re-fetched for an unassociated PR run." | ||
| fi | ||
| if ! jq -e ' | ||
| all(.[]; | ||
| (.repo | type) == "string" and (.repo | length) > 0 and | ||
| (.ref | type) == "string" and (.ref | length) > 0 | ||
| ) | ||
| ' <<<"$fresh_open_pr_refs_json" >/dev/null; then | ||
| warn_preserve "fresh open PR head evidence is malformed." | ||
| fi | ||
| if jq -e \ | ||
| --arg repo "$run_head_repo" \ | ||
| --arg ref "$run_branch" \ | ||
| 'any(.[]; .repo == $repo and .ref == $ref)' \ | ||
| <<<"$fresh_open_pr_refs_json" >/dev/null; then | ||
| encoded_run_ref="$(encode_ref_path "$run_branch")" | ||
| if ! final_ref_sha="$( | ||
| gh api \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| "/repos/${run_head_repo}/git/ref/heads/${encoded_run_ref}" \ | ||
| --jq '.object.sha // empty' | ||
| )"; then | ||
| warn_preserve "live ref for newly associated PR head could not be re-fetched before cancellation." | ||
| fi | ||
| if ! [[ "$final_ref_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then | ||
| warn_preserve "live ref for newly associated PR head is malformed." | ||
| fi | ||
| if [ "$run_head" = "$final_ref_sha" ]; then | ||
| warn_preserve "run became associated with an open PR at its authoritative current head after queue classification." | ||
| fi |
There was a problem hiding this comment.
📝 Info: Unassociated runs avoid stale payloads
Late association matches repository and branch identity, then resolves the Git ref. A stale pull-request list SHA cannot authorize cancellation.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Root cause
The organization queue sweep treated the SHA in the open-PR list as the
authoritative branch head. During a synchronize-event propagation window,
LineageWeave #667 already had current-head runs for
37cc9ab1, while the PRpayload still exposed its preceding head. Queue hygiene cancelled the valid new
runs as superseded.
Repair
Get a referenceendpointEvidence
uv run --group dev pytest -q— 1,437 passed, 1 skipped, 16 subtests passedcoverage run -m pytest -q tests && coverage report --fail-under=100— 100% statements and branchesinterrogate --fail-under=100 scripts/ci— 100%actionlint .github/workflows/pr-review-merge-scheduler.ymlgit diff --checkCross-repository incident: ContextualWisdomLab/LineageWeave#667.
Summary by CodeRabbit
버그 수정
문서
테스트