t4527: prevent pulse redispatch for already-merged tasks#4549
t4527: prevent pulse redispatch for already-merged tasks#4549marcusquinn merged 3 commits intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughIntroduces a third dedup layer detecting merged PRs to prevent task re-dispatch. Updates pulse orchestration documentation with comprehensive guard rails, priority scheduling, quality-debt workflows, and audit requirements. Implements merged-PR detection function and extends dispatch validation logic with corresponding test coverage. Changes
Sequence DiagramsequenceDiagram
participant Supervisor as Supervisor<br/>(pulse)
participant Wrapper as Pulse Wrapper<br/>Scripts
participant GitHub as GitHub API
participant Queue as Task Queue
Supervisor->>Wrapper: check_dispatch_dedup(repo, issue)
Wrapper->>Wrapper: Layer 1: Check exact<br/>repo+issue match
alt Already dispatched
Wrapper-->>Supervisor: SKIP (dedup exact)
end
Wrapper->>Wrapper: Layer 2: Check<br/>normalized title
alt Title match found
Wrapper-->>Supervisor: SKIP (dedup title)
end
Wrapper->>Wrapper: Layer 3: has_merged_pr_for_issue()
Wrapper->>GitHub: gh pr list --search<br/>"closes `#NNN`" --state merged
GitHub-->>Wrapper: [Merged PR results]
alt Merged PR detected
Wrapper->>Wrapper: Extract task-id from issue<br/>and search PR titles
GitHub-->>Wrapper: [Task-id match results]
end
alt Any merged PR found
Wrapper-->>Supervisor: SKIP (dedup merged PR)
else No merged PR evidence
Wrapper-->>Supervisor: PROCEED to dispatch
Supervisor->>Queue: Enqueue worker
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Mar 14 00:49:47 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
marcusquinn
force-pushed
the
bugfix/4527-merged-pr-redispatch-guard
branch
from
f6687c3 to
edbf320
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Mar 14 01:09:07 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.agents/scripts/commands/pulse.md (1)
524-545: Prefercheck_dispatch_dedupover re-stating its subchecks in the prompt.This section re-encodes wrapper-owned logic even though the snippet already sources the helper that centralizes it. Using the consolidated function keeps the prompt and
.agents/scripts/pulse-wrapper.shaligned when dedup rules change again.♻️ Collapse the prompt to the single shell helper
-# Check 1: exact repo+issue match via process list -if has_worker_for_repo_issue <number> <slug>; then - echo "Worker already running for #<number> in <slug> — skipping" - continue -fi - -# Check 2: normalized dedup via dispatch-dedup-helper (catches title variants) -if ~/.aidevops/agents/scripts/dispatch-dedup-helper.sh is-duplicate "Issue #<number>: <title>"; then - echo "Duplicate dispatch detected for #<number> — skipping" - continue -fi - -# Check 3: merged-PR evidence (direct close keyword OR task-id fallback) -if has_merged_pr_for_issue <number> <slug> "<issue title>"; then - echo "Issue #<number> already has merged PR evidence — skipping dispatch" +if check_dispatch_dedup <number> <slug> "Issue #<number>: <title>" "<issue title>"; then + echo "Duplicate or already-merged work detected for #<number> — skipping dispatch" continue fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/commands/pulse.md around lines 524 - 545, The snippet duplicates the wrapper's dedup logic (has_worker_for_repo_issue, dispatch-dedup-helper.sh is-duplicate, has_merged_pr_for_issue) instead of using the centralized helper; replace the three separate checks with a single call to check_dispatch_dedup (provided by sourcing pulse-wrapper.sh) so the script uses the canonical dedup rules defined in pulse-wrapper.sh and stays in sync when dedup behavior changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/pulse-wrapper.sh:
- Around line 2193-2201: Update the keyword loop that builds the search query
for PRs so it checks all GitHub auto-close variants (close, closes, closed, fix,
fixes, fixed, resolve, resolves, resolved) instead of only "closes fixes
resolves"; modify the for-loop that iterates over keyword (which constructs
query="${keyword} #${issue_number} in:body" and uses gh pr list -> pr_json and
pr_count) to iterate the full nine-word set so merged PRs using singular or
past-tense forms are detected and the dedup guard prevents re-dispatch.
In @.agents/scripts/tests/test-pulse-wrapper-worker-detection.sh:
- Around line 87-120: The gh() stub currently only matches on the --search value
from GH_SEARCH_FIXTURES which lets has_merged_pr_for_issue() call gh pr list
with the wrong --repo or --state and still pass; update the gh() function to
parse and validate --repo and --state flags in addition to --search (or
construct fixture keys that include repo and state), match against a compound
key in GH_SEARCH_FIXTURES (e.g. "repo|state|search") inside gh(), and if
unexpected or missing --repo/--state values are provided, print a clear error
and return non‑zero so tests fail loudly; refer to the gh() stub,
GH_SEARCH_FIXTURES, and the has_merged_pr_for_issue() test call when making
these changes.
---
Nitpick comments:
In @.agents/scripts/commands/pulse.md:
- Around line 524-545: The snippet duplicates the wrapper's dedup logic
(has_worker_for_repo_issue, dispatch-dedup-helper.sh is-duplicate,
has_merged_pr_for_issue) instead of using the centralized helper; replace the
three separate checks with a single call to check_dispatch_dedup (provided by
sourcing pulse-wrapper.sh) so the script uses the canonical dedup rules defined
in pulse-wrapper.sh and stays in sync when dedup behavior changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6f8d14c-eefe-4dab-959e-97dd4573bae4
📒 Files selected for processing (3)
.agents/scripts/commands/pulse.md.agents/scripts/pulse-wrapper.sh.agents/scripts/tests/test-pulse-wrapper-worker-detection.sh
- Expand has_merged_pr_for_issue keyword loop to all 9 GitHub auto-close variants (close/closes/closed, fix/fixes/fixed, resolve/resolves/resolved) - Update gh() test stub to validate --repo and --state flags alongside --search using compound fixture keys (repo|state|query|payload format) - Update all fixture strings in test functions to use compound key format - Collapse three separate dedup if-blocks in pulse.md into single check_dispatch_dedup call (nitpick) - All 7 tests pass; shellcheck clean on both modified .sh files
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Mar 14 01:25:00 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
… flags The previous commit added compound key matching but the stub still silently returned [] when --repo or --state were missing. Now it fails loudly with a diagnostic message, catching regressions where has_merged_pr_for_issue() omits required flags.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Mar 14 03:16:10 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
|
Pulse triage: CHANGES_REQUESTED from CodeRabbit. Fix needed in |
|
All CHANGES_REQUESTED review feedback addressed:
All 7 tests pass, shellcheck clean. @coderabbitai review |
|
Kicking off a fresh review now. 🧠 Learnings used✅ Actions performedReview triggered.
|
2 participants
Summary
pulse-wrapper.shso dispatch dedup skips issues already resolved by merged PRs.Closes #4527
Summary by CodeRabbit
New Features
Tests