Skip to content

fix(#1408): deduplicate retro dispatches on PR merge - #1412

Closed
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1408-dedup-retro-dispatch
Closed

fix(#1408): deduplicate retro dispatches on PR merge#1412
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1408-dedup-retro-dispatch

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

When a PR is merged, multiple queued shim runs (each using cancel-in-progress: false) independently process the closed event and dispatch a retro workflow. While retro.yml already has a concurrency group with cancel-in-progress: true, retro runs may complete before the next dispatch arrives, making cancellation ineffective and producing 2-6 duplicate retro runs per merge.

Add a dedup check in the dispatch step: before dispatching a retro workflow, query the GitHub API for queued or in-progress retro runs with matching source repo and PR number. If a match is found, skip the dispatch with a notice annotation. The check is fail-open — API errors fall through to normal dispatch to avoid blocking legitimate retro runs.

Note: Go tests could not run (sandbox has Go 1.23, project requires 1.26). The change is workflow YAML only and does not affect Go source. Pre-commit could not complete (Go toolchain download blocked in sandbox). Manual verification required.


Closes #1408

Post-script verification

  • Branch is not main/master (agent/1408-dedup-retro-dispatch)
  • Secret scan passed (gitleaks — 6cdb528b675d3c97499252b6a11606631ecc127e..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

When a PR is merged, multiple queued shim runs (each using
cancel-in-progress: false) independently process the closed
event and dispatch a retro workflow. While retro.yml already
has a concurrency group with cancel-in-progress: true, retro
runs may complete before the next dispatch arrives, making
cancellation ineffective and producing 2-6 duplicate retro
runs per merge.

Add a dedup check in the dispatch step: before dispatching a
retro workflow, query the GitHub API for queued or in-progress
retro runs with matching source repo and PR number. If a match
is found, skip the dispatch with a notice annotation. The
check is fail-open — API errors fall through to normal
dispatch to avoid blocking legitimate retro runs.

Note: Go tests could not run (sandbox has Go 1.23, project
requires 1.26). The change is workflow YAML only and does not
affect Go source. Pre-commit could not complete (Go toolchain
download blocked in sandbox). Manual verification required.

Closes #1408
@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://ad3ebc4d-site.fullsend-ai.workers.dev

Commit: e03dc6184c28c0abf033fca18228cf8422eed6f3

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

No findings.

The dedup logic is correctly scoped to STAGE == "retro", extracts the PR number from the same EVENT_PAYLOAD structure built earlier in the step, and compares source_repo + PR number against active retro.yml runs. The fail-open design (|| true on API calls) ensures dedup failures never block legitimate dispatches. The inherent TOCTOU race (multiple shim runs checking simultaneously) is mitigated by retro.yml's existing cancel-in-progress: true concurrency group. The exit 0 on match correctly skips the entire dispatch step, which is safe since only retro-stage workflows would be dispatched when STAGE == "retro". The lint-max-lines bump from 390→415 accommodates the 24 new lines.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label May 25, 2026
@rh-hemartin

Copy link
Copy Markdown
Member

/fix implement the preferred method described in the linked issue: "Preferred: Add a concurrency group in the retro.yml workflow keyed on the originating PR URL/number with cancel-in-progress: true. This way, if multiple retro dispatches arrive for the same PR, only the last one runs."

@rh-hemartin rh-hemartin self-assigned this May 26, 2026
@rh-hemartin

Copy link
Copy Markdown
Member

/fs-fix implement the preferred method described in the linked issue: "Preferred: Add a concurrency group in the retro.yml workflow keyed on the originating PR URL/number with cancel-in-progress: true. This way, if multiple retro dispatches arrive for the same PR, only the last one runs."

@rh-hemartin

Copy link
Copy Markdown
Member

Replaced by #1468

@rh-hemartin
rh-hemartin deleted the agent/1408-dedup-retro-dispatch branch May 26, 2026 08:06
rh-hemartin added a commit that referenced this pull request May 26, 2026
Adds a 60-second debounce job before the retro job. Multiple dispatches
for the same PR close (from shim retries or burst webhook deliveries)
start simultaneously but the sleep keeps runs active long enough for the
existing cancel-in-progress: true concurrency group to cancel stale ones.
Only the last run survives the debounce and proceeds.

Supersedes #1412, which used a pre-dispatch API dedup check instead.
Closes #1408.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
rh-hemartin added a commit that referenced this pull request May 28, 2026
Adds a 60-second debounce job before the retro job. Multiple dispatches
for the same PR close (from shim retries or burst webhook deliveries)
start simultaneously but the sleep keeps runs active long enough for the
existing cancel-in-progress: true concurrency group to cancel stale ones.
Only the last run survives the debounce and proceeds.

Supersedes #1412, which used a pre-dispatch API dedup check instead.
Closes #1408.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deduplicate retro dispatches on PR merge/close events

1 participant