search

CircleCI-Public / orb-tools-orb

Various tools for authoring and publishing CircleCI orbs
https://circleci.com/orbs/registry/orb/circleci/orb-tools
MIT License
51 stars 74 forks source link

Orb builds spam comments into PRs after PR has been merged #186

Closed Peter-Darton-i2 closed 10 months ago

Peter-Darton-i2 commented 1 year ago

Orb version:

orbs:
  orb-tools: circleci/orb-tools@11

...although the issue is still present in @12 as well.

...

# Orb 'circleci/orb-tools@11' resolved to 'circleci/orb-tools@11.6.0'

...although the issue is still present in @12.0.4 as well.

What happened:

Each time my main branch builds, I get a comment added to the last github PR that was merged, causing (unwanted and annoying) email spam & (unwanted but ignorable) comment spam in the PR.

e.g. this build of my "experimental stuff" orb's main branch added a new comment saying "Your development orb has been published. It will expire in 30 days. You can preview what this will look like on the CircleCI Orb Registry at the following link: ..." to the last PR that was merged in, https://github.com/i2group/circleci-orb-organize-me/pull/8

This seems to be a flaw in the orb-tools orb's publish job - it doesn't seem to know when to stop commenting. image

Expected behavior:

While it's 100% desirable for a PR-build (i.e. a build of the PR's branch) to add a comment about the PR-build's dev-only publication of the orb, it is NOT desirable for non-PR builds to go commenting on PRs. image

The only builds that should append comments to GitHub PR are builds of that PR's branch.

Additional Information:

Typical repro-case is:

  1. have an orb with source in github
  2. make a PR containing a single commit
  3. allow that to build
  4. see that there's a (desired) comment in the PR's comment history regarding the dev publish
  5. see that github has emailed you informing you of this comment
  6. ensure that the PR is fully up to date w.r.t. the main branch (update the PR branch if needs me and allow CCI to rebuild)
  7. merge the PR "as-is", causing the PR's sole commit to be added to the main branch's head unaltered
  8. wait for CCI to build the main branch
  9. see that there's now another (this time unwanted) comment in the now-merged-and-closed PR.
  10. see that github has spammed your email about the unwanted comment
  11. do a release of the orb by adding a tag etc
  12. wait for CCI to build the tag and officially release the new orb version
  13. ...but see that there's yet another unwanted comment in the merged PR.
  14. ...and more email spam

For extra spam, schedule (e.g. weekly) additional rebuilds of your orb's main branch (we wanted this so we'll get a fair warning if any external dependency updates break the orb) and then you'll get spammed on schedule too 😛

I suspect that the code is blindly assuming that it should comment on any PR whose branch contains the git commit it just built. This would be OK if git commit SHAs were guaranteed to be unique to where they came from, but this isn't the case, e.g. if the commit that gets merged into main is precisely the same as the last commit in the PR branch. There's a large variety of reasons why the commit into main won't be 100% identical to the last commit from a PR branch (e.g. if doing a squash of multiple commits from the PR branch into just one onto main) but if the orb maintainer "gets it 100% right on their first and only commit" then it's far more likely that the last commit to main will be === the last commit in the PR that was last merged ... which then results in a false-match in the "add comment to PR" code, which then results in comments being added where they're not wanted, which then results in email spam.

Peter-Darton-i2 commented 12 months ago

Note that while this issue was first found in v11 of the orb, it's still present in v12. It looks like #215 would be a very neat fix for this issue.