Skip to content

Hold an Attested Wait Until the Base Branch's Required Checks Register - #2837

Merged
ptr727 merged 4 commits into
developfrom
feature/pr-review-wait-required-checks
Oct 11, 2026
Merged

ptr727 merged 4 commits into
developfrom
feature/pr-review-wait-required-checks

Conversation

@ptr727

@ptr727 ptr727 commented Oct 11, 2026

Copy link
Copy Markdown
Owner

pr_review.py wait on an attested head returned exit 0 while the base branch's other required checks had not yet registered, and attest refused straight after a push.

  • wait now reads the required status contexts from the base branch's rules (once per run) and holds a BLOCKED merge while one is absent from a whole-read rollup. An unreadable rules read keeps the old result and prints a note.
  • attest waits up to about 60 seconds for GitHub to register a push, only where the remote branch tip equals the checkout's HEAD and the pull request is open.
  • Local strict review ran two rounds. Accepted and not changed: a required context that never registers holds until the timeout (the settled design), the ls-remote against the head repository URL, and gql exiting on a GraphQL error in the new branch read.

Closes #2750

🤖 Generated with Claude Code

ptr727 and others added 3 commits October 11, 2026 12:36
The wait read a BLOCKED merge with every registered check passed as nothing left to
wait for, because the rollup only lists checks that have registered. It now reads the
required contexts from the base branch's rules and holds while one is absent.

Attest also waits up to about a minute for GitHub to move the pull request's head to a
push whose remote tip is the checkout's own commit, instead of refusing at once.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Review of the first commit found the hold fired on an unreadable or truncated rollup,
read the rules on every covered head, skipped the retry schedule, and called a head that
moved to a third commit unregistered. Each is fixed and tested, and the README states the
new pending case.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A closed or merged pull request never moves its head, so waiting a minute for it and
then calling the push unregistered would mislead. It now refuses at once as before.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 11, 2026 19:49
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8db6d0c3-5641-44f2-a78a-fa844bf83d3f

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.47%. Comparing base (4035555) to head (dcdfaf2).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2837      +/-   ##
===========================================
+ Coverage    60.26%   60.47%   +0.20%     
===========================================
  Files           16       16              
  Lines         8489     8528      +39     
===========================================
+ Hits          5116     5157      +41     
+ Misses        3373     3371       -2     
Flag Coverage Δ
python-3.13 60.47% <100.00%> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Required-check integration matching and slash-containing ref handling remain unresolved.

2 open findings
What changed in this PR

Updates pr_review.py to wait for delayed required-check registration and attestation readiness.

Changes:

  • Reads required checks from base-branch rules.
  • Adds bounded polling before attestation.
  • Adds regression tests and documentation updates.

Review findings:

  • Critical: Required-check matching must preserve integration bindings (1 vote).
  • Moderate: Slash-containing base refs must be encoded for rules lookup (4 votes).
File Description
tests/​test_pr_review.py Tests delayed registration and rules handling.
scripts/​README.md Documents the updated wait behavior.
scripts/​pr_review.py Implements rules-based check holding and attestation polling.

🧠 Review effort: Lite


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/pr_review.py
Comment thread scripts/pr_review.py Outdated
A base such as release/1.0 now travels as one explicit path segment, so the rules read
cannot depend on how the API splits a raw slash.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@ptr727

ptr727 commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head dcdfaf25915137590a1bf1363cd5e25bd7e159b2, the content this pull request carries at that commit against develop, and it recorded 1 finding.

@ptr727
ptr727 merged commit 9b02661 into develop Oct 11, 2026
13 checks passed
@ptr727
ptr727 deleted the feature/pr-review-wait-required-checks branch October 11, 2026 20:06
ptr727 added a commit that referenced this pull request Oct 11, 2026
…ted Wait Hold to Main (#2840)

## Summary

Promotes [#2836](#2836)
and [#2837](#2837) to
main.

- `prose_lint.py`'s opt-in `sentence-length` check now reads code
comments outside Markdown and Python docstrings, scoped to changed lines
the way Markdown already was. The `comment-and-doc-style` Skill names
the wider scope.
- `pr_review.py wait` holds an attested head on a BLOCKED merge while a
check the base branch's rules require has not registered. An unreadable
rules read keeps the old behavior and says so. `attest` waits up to 60
seconds for a push GitHub has not registered yet, only where the remote
branch already holds the checkout's commit.

Follow-ups filed: #2838 (sentence-length gaps past the edit budget) and
#2839 (required-check matching ignores the rule's `integration_id`).

Closes #2430
Closes #2750

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants