spr up seems to always trigger duplicate synchronize webhooks

[jsravn]

Hi, I'm enjoying this project a lot. It really helps the dev workflow on github. Thanks!

I'm encountering one issue - whenever I do a git spr up on an existing PR, it causes github to emit two identical synchronize webhooks. As a result, my CI system runs two duplicate builds.

AFAIK synchronize should only be issued when the source branch is updated. But whatever spr is doing under the hood causes github to issue it twice. Oddly, the contents are identical - even the headers are the same, except for the X-GitHub-Delivery value. (actually, not always, sometimes there is a 1 second difference in the pushed_at fields).

Here is an example log output of when it happens

❯ git spr up --detail
> git rev-parse --show-toplevel
> git fetch
> git rebase origin/main --autostash
> github fetch pull requests
> git branch
> git log origin/main..HEAD
> git status --porcelain --untracked-files=no
> git push --force --atomic origin 12461fb8173a3372c5f2d92c049525137edcf40a:refs/heads/pr/jsravn/fix-cd-controller-deploy/7cb948cd
> github update 545 - cd: Fix cd-controller role to be namespaced
> github fetch pull requests
> git branch

Might be the git push followed by the github update that causes it?

[ejoffe]

You should be able to remove the github update call to test out your theory. This call is there to update the stack part of the pr message body but it is not functionally required, so I think everything should still work ok. If this is the case, we can add a feature flag to disable the stack display of the pr message body.

index fa72383..d2e634c 100644
--- a/spr/spr.go
+++ b/spr/spr.go
@@ -191,7 +191,8 @@ func (sd *stackediff) UpdatePullRequests(ctx context.Context, reviewers []string
        sd.profiletimer.Step("UpdatePullRequests::updatePullRequests")

        for _, pr := range updateQueue {
-               sd.github.UpdatePullRequest(ctx, githubInfo, pr.pr, pr.commit, pr.prevCommit)
+               //      sd.github.UpdatePullRequest(ctx, githubInfo, pr.pr, pr.commit, pr.prevCommit)
+               _ = pr
        }

        sd.profiletimer.Step("UpdatePullRequests::commitUpdateQueue")```

[jsravn]

Thanks, I'll give it a try. It sort of feels like a github bug to be honest - maybe there is a race between the api and the git push.

[ejoffe]

pro tip: If you have goreleaser installed, you can build the project using make bin I alias my dev built spr so I can easily test it: alias spr='/Users/eitan/Code/spr/dist/spr_darwin_amd64/git-spr'

[dan-v]

I also ran into this duplicate issue with a CI workflow that had both push and pull_request events configured:

on:
  push:
    branches: [master]
  pull_request:
    branches: [master]

As spr does a push followed by an update of the pull request, as mentioned above, it was triggering this workflow twice. What actually happens is the first event fires on push, then the second event fires on PR update, the first workflow ends up getting cancelled while the second runs and succeeds. This ends up breaking spr 'github checks', as it's looking at the first cancelled workflow rather than the secondary run that succeeded, so you end up with a big red X for spr status even though the PR is really green.

For now, I ended up switching to triggering on all push events instead, which seems to works ok for this workflow as it still triggers for PRs and merges to master.

on:
  push:

[ejoffe]

Looks like the right approach is to use push events rather than pull_request events. Could be worth while to add this into the readme somewhere.

[annervisser]

@jsravn Did you ever figure out the issue? I have a tool that does pretty much exactly the same (git push followed by gh pr edit), and I'm observing the same behaviour where the pull_request synchronize event is triggered twice.