danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.26k stars 368 forks source link

[BUG] Duplicate report coments on GitHub actions if we use a custom CI bot token #1347

Closed snitin315 closed 1 year ago

snitin315 commented 1 year ago

Describe the bug

Github-actions[bot] user id is hardcoded in the codebase:

https://github.com/danger/danger-js/blob/47dfcfa1bb454199452ab021bcbd594eb1efa8f1/source/platforms/github/GitHubAPI.ts#L133-L139

So if we do not use Github-actions[bot] on CI we get duplicate comments as the following condition is always false: https://github.com/danger/danger-js/blob/47dfcfa1bb454199452ab021bcbd594eb1efa8f1/source/platforms/github/GitHubAPI.ts#L93

As a result, we get duplicate comments on the PR.

We have a separate bot account on GitHub which we use to perform all the automation via GitHub actions, hence not using the default secrets.GITHUB_TOKEN on CI.

To Reproduce

Run dangerJS on GitHub actions with a token other than secrets.GITHUB_TOKEN

Expected behavior

It should edit the previous comment.

Screenshots

Screenshot 2023-01-09 at 8 31 24 AM

Your Environment

software version
danger.js 11.2.1
node 18
npm 8
Operating System

Additional context

Allow a new env variable USE_CUSTOM_CI_TOKEN or something similar.

orta commented 1 year ago

I agree that your logic is pretty sound, but the thing that doesn't fit for me is that almost everywhere I use danger it's GitHub Actions + a custom token - for example here: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/63851#issuecomment-1375044192

I think it's likely you're not using DANGER_GITHUB_API_TOKEN which takes priority at a guess

snitin315 commented 1 year ago

@orta So earlier I was using GITHUB_TOKEN: custom-token which I updated to DANGER_GITHUB_API_TOKEN: custom-token and I'm still seeing duplicate comments.

In the example you showed in https://github.com/DefinitelyTyped/DefinitelyTyped/pull/63851#issuecomment-1375044192, it seems to be using danger v10.1.1 whereas the above logic was introduced later. I updated the danger-js package on CI and was able to reproduce:

Screenshot 2023-01-09 at 3 51 41 PM
mrclrchtr commented 1 year ago

We have the same problem. We updated today from 11.1.4 to 11.2.1 and since then the bot writes new comments instead of editing old ones. We have always used DANGER_GITHUB_API_TOKEN.

orta commented 1 year ago

https://github.com/danger/danger-js/pull/1337 feels like the culprit to me

snitin315 commented 1 year ago

@orta Should we revert the PR then?

martinerko commented 1 year ago

@orta I am experiencing the same issue, is there any plan to revert the problematic commit?

orta commented 1 year ago

I figured someone would have a think and see if my guess was correct - but I'm fine with just reverting it

orta commented 1 year ago

Alright, that's shipped

snitin315 commented 1 year ago

@orta Thanks. I just updated to v11.2.2 and it has fixed the issue.