[wip] #28 automation: handle stale CHANGES_REQUESTED gate after bot re-review - #51
Conversation
📝 WalkthroughWalkthroughThe issue-28 kickoff document was restructured to focus on handling stale CHANGES_REQUESTED gates. It now defines a problem statement about deadlock conditions, outlines four focused implementation slices with specific checks and actions, and provides acceptance criteria with concrete next steps for testing and development. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
|
Synced this branch with latest main to clear merge conflicts (merged main into , resolved kickoff-plan conflict, pushed ). CI should re-run automatically. |
|
Follow-up: branch is now synced with latest main via merge commit 25e15b0; merge conflict in plans/issue-kickoff/issue-28.md was resolved and pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plans/issue-kickoff/issue-28.md (2)
10-18: Tighten stale-review comparator rules for deterministic behavior.Good slice definition, but the evaluator should explicitly define tie-breaks (e.g., equal timestamps) and normalized time basis to avoid ambiguous stale/non-stale outcomes in edge cases. Consider adding one bullet with the exact comparison contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plans/issue-kickoff/issue-28.md` around lines 10 - 18, Update the stale-review comparator to use a precise comparison contract: normalize all timestamps to UTC milliseconds from the same source (e.g., commit author/committer date and review submitted_at) before comparing, then mark stale only when required-checks are green AND headCommitTimestamp > latestActionableReviewTimestamp (strict greater-than); if timestamps are equal treat as not-stale (no action). Ensure the comparator function (stale-review comparator / helper that reads latest actionable review timestamp, latest head commit timestamp, and required-checks all-green state) enforces this normalization and strict comparison and documents the tie-break rule in its comment.
20-22: Specify concrete wait-loop bounds and retry cadence.“Bounded wait + poll loop” is the right direction, but this should include explicit max attempts/timeout and poll interval/backoff so operators get predictable latency and failure behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plans/issue-kickoff/issue-28.md` around lines 20 - 22, Update the "Bounded wait + poll loop" plan to specify concrete retry parameters: add explicit names and values for maxAttempts (or overall timeout), initialPollInterval (ms), and backoffFactor (or fixed interval) and describe the terminal condition that transitions to the "needs-human-unblock" path; reference the "Trigger one re-review request/ping path" as the single automatic retry action executed once before starting the bounded poll loop, then describe the poll loop behavior (initial interval, backoff, maxAttempts/timeout) and the exact condition that causes fallback to needs-human-unblock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plans/issue-kickoff/issue-28.md`:
- Around line 10-18: Update the stale-review comparator to use a precise
comparison contract: normalize all timestamps to UTC milliseconds from the same
source (e.g., commit author/committer date and review submitted_at) before
comparing, then mark stale only when required-checks are green AND
headCommitTimestamp > latestActionableReviewTimestamp (strict greater-than); if
timestamps are equal treat as not-stale (no action). Ensure the comparator
function (stale-review comparator / helper that reads latest actionable review
timestamp, latest head commit timestamp, and required-checks all-green state)
enforces this normalization and strict comparison and documents the tie-break
rule in its comment.
- Around line 20-22: Update the "Bounded wait + poll loop" plan to specify
concrete retry parameters: add explicit names and values for maxAttempts (or
overall timeout), initialPollInterval (ms), and backoffFactor (or fixed
interval) and describe the terminal condition that transitions to the
"needs-human-unblock" path; reference the "Trigger one re-review request/ping
path" as the single automatic retry action executed once before starting the
bounded poll loop, then describe the poll loop behavior (initial interval,
backoff, maxAttempts/timeout) and the exact condition that causes fallback to
needs-human-unblock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b52dd2f0-213c-4925-85b2-84593a5847cf
📒 Files selected for processing (1)
plans/issue-kickoff/issue-28.md
2 participants
Kickoff implementation refresh for #28 with concrete stale-review detection slices and acceptance mapping.\n\nThis PR is focused on implementation planning updates in-branch so the next commit can land evaluator logic + tests.\n\nFixes #28
Summary by CodeRabbit