New Workflow to be used with Discord Webhook

[Hecter94]

Feature: This PR implements a new workflow to post new commits to the community Discord automatically

Feature Details

With the addition of this new workflow, we can host a commit-stream channel to keep the community as a whole and content creators informed of any changes. This has been an oft-requested feature.

Using a workflow rather than an external service avoids needing to allow external service access to the repository, which is beneficial for security purposes. It also allows us complete control over what content is displayed, which would not be possible using the built-in GitHub webhook.

Screenshots

An example of how a commit would look when posted to Discord; It is currently showing the author's name and commit message.

Requirements for merge

When merging this PR, two additional updates will need to be made to the Endless Sky repo in order for it to work correctly.

1. An admin will need to add the discord webhook URL as a new environment variable, which I will provide privately to keep it secret.
2. Our current commit log webhook should be turned off to avoid duplicating new commit messages.

Testing Done

I have tested this workflow and confirmed that it ran and posted correctly when using a private fork of the repository.

[tehhowch]

With the addition of this new workflow, we can host a commit-stream channel to keep the community as a whole and content creators informed of any changes. This has been an oft-requested feature.

We can and apparently do already use the github push webhook, so "post commits to discord" isn't a reason to merge the PR:

Requirements for merge

2. Our current commit log webhook should be turned off to avoid duplicating new commit messages.

Using a workflow rather than an external service avoids needing to allow external service access to the repository, which is beneficial for security purposes.

But then we go and add a new action, which can have its own security issues😉

It also allows us complete control over what content is displayed, which would not be possible using the built-in GitHub webhook.

Screenshots

An example of how a commit would look when posted to Discord; It is currently showing the author's name and commit message.

This is the bit that is differentiating IMO. How useful is it, vs the default formatting, given the information available in the commit webhook payload? I don't personally find a list of commits that interesting, so doing work to format them differently isn't very appealing.

[Hecter94]

We can and apparently do already use the github push webhook, so "post commits to discord" isn't a reason to merge the PR:

The one we have set up now was just for me to test. It's not posted publicly in any way, this PR is a continuation of the testing that I started doing with that default webhook 😉

But then we go and add a new action, which can have its own security issues😉

True, but when we previously looked into something like IFTTT or Zapier, it was essentially looking for Admin-level access, which none of us felt comfortable with. Specifically, it was asking for the following:

This is the bit that is differentiating IMO. How useful is it, vs the default formatting, given the information available in the commit webhook payload? I don't personally find a list of commits that interesting, so doing work to format them differently isn't very appealing.

This is the main advantage I would say of doing it via an action rather than the default.
The default webhook has a couple of issues:

1. Because it is set to trigger on any push event, the push of our continuous tag causes the default webhook to fire twice, resulting in two duplicate messages whenever something is committed. With my testing, this doesn't seem to happen using this action.
2. Long commit titles using the default format cut off the commit description, eg.
   
3. The default format includes a lot of extra information. It embeds the message, resulting in the basic information, e.g. "Commit author" and "commit message," getting lost among information your standard user isn't interested in.
4. This may not be relevant to this PR getting merged, but this does open up a lot of possibilities for us to include additional information in the commit messages that could be useful above and beyond what the default commit message shows.

My apologies; looking back, this is all information I should have included in the PR description for clarity.

[MasterOfGrey]

There's also a lot of things you can do with the custom webhook - like ping reviewers when new PR's are created - which are simply unsupported by the default webhook functionality in Github.
In general the default is actually very limited tbh

[tehhowch]

There's also a lot of things you can do with the custom webhook - like ping reviewers when new PR's are created - which are simply unsupported by the default webhook functionality in Github.
In general the default is actually very limited tbh

To be fair, GitHub's webhooks aren't intended to do crazy things -- they're a notification service for a particular activity. They send a blob of data about the event, and then it's on the receiver to do cool things with the information. Discord has kindly provided a basic template for displaying the JSON, and anyone who wants to do something more needs to do the work. It usually means your own server and integrating with the associated client libraries to do whatever it is you want.

IMO I would rather we forward the relevant events to our existing Discord bot and have it perform the necessary actions, rather than add a new workflow. Configuring James to tag a user group when a particular label is newly applied to an open PR should not be too difficult.

@MCOfficer thoughts?

[MCOfficer]

I came up with this idea, because writing a discord bot to receive, understand, filter & pass on events webhooks is

• a bunch of work that could be avoided
• introducing bus factor due to hosting & bot knowledge
• introducing risk of downtime due to hosting

Whereas github actions are open-source, several handful people in the community know how to edit them if needed, and have no bus-factor or risk beyond our repo's existing one.

For clarity, these were the other options:

• Host your own kind of IFTTT (Huginn), carries the same risks regarding hosting & configuration
• Use IFTTT or Zapier: Possible, but seems to give those services a stupid amount of access to our repo. More than Derpy has, is what i'm saying. I trust IFTTT, but i still felt very uneasy about that.

[Hecter94]

Would it be possible to use both a GitHub action and a Discord bot?

We could use the action to build a usefully formatted message that avoids the issues I mentioned above, specifically:

1. Because it is set to trigger on any push event, the push of our continuous tag causes the default webhook to fire twice, resulting in two duplicate messages whenever something is committed. With my testing, this doesn't seem to happen using this action.
2. Long commit titles using the default format cut off the commit description.

The discord bot would then be available to do more advanced actions, like reacting to the message, opening threads, pinging various groups etc.

The main advantages are:

1. The bot can be much simpler since it's just reading and editing discord messages rather than accepting, interpreting and formatting webhook payloads.
2. If the bot ever goes down or stops working, we still have usefully formatted messages rather than nothing.
3. More people can make changes to the GH Action to edit it without needing to edit the bot directly.
4. The action can be implemented immediately, with the more advanced actions being built into the bot later.

[Zitchas]

Just to note: I have put in place the secret DISCORD_WEBHOOK that this refers to.

[tehhowch]

I came up with this idea, because writing a discord bot to receive, understand, filter & pass on events webhooks is

• a bunch of work that could be avoided
• introducing bus factor due to hosting & bot knowledge
• introducing risk of downtime due to hosting

You're correct on all these counts. James would need some attention from a DevOps perspective to limit the impacts, including (egads) automated testing and at least an on-demand deploy workflow. I don't know any of the bot's current metrics to determine which tiers of various hosting providers we might fall into.

Whereas github actions are open-source, several handful people in the community know how to edit them if needed, and have no bus-factor or risk beyond our repo's existing one.

Also not wrong. But we are limited by the available Marketplace actions w.r.t. what we can do with the workflow.

For clarity, these were the other options:

• Host your own kind of IFTTT (Huginn), carries the same risks regarding hosting & configuration
• Use IFTTT or Zapier: Possible, but seems to give those services a stupid amount of access to our repo. More than Derpy has, is what i'm saying. I trust IFTTT, but i still felt very uneasy about that.

I agree on not using ifttt / zapier.

Would it be possible to use both a GitHub action and a Discord bot?
The discord bot would ... do more advanced actions, like reacting to the message, opening threads, pinging various groups etc.

The main advantages are:

1. The bot can be much simpler since it's just reading and editing discord messages rather than accepting, interpreting and formatting webhook payloads.
2. If the bot ever goes down or stops working, we still have usefully formatted messages rather than nothing.
3. More people can make changes to the GH Action to edit it without needing to edit the bot directly.
4. The action can be implemented immediately, with the more advanced - actions being built into the bot later.

Yes, but it isn't ideal, as we lose a lot of information on the handoff. When James receives the webhook directly, we can use all attributes of the JSON payload to dispatch the correct behavior. If James is only listening to a particular channel for a post by a particular user, all we have available is the content of that post and its authorship -- anything we want to do with James has to be drivable solely from that. Any changes to content of the action's post would need to ensure that James still gets all the information needed, which would likely be link parsing and then one or more calls to the GitHub API for attributes available from the referenced PR or branch + sha.
I'm not saying we would need no calls to the GitHub API if James did it all, but we would definitely have more information available and it would not be susceptible to users editing the formatting but not knowing about the Discord integration.

We could use the action to build a usefully formatted message that avoids the issues I mentioned above, specifically:

1. Because it is set to trigger on any push event, the push of our continuous tag causes the default webhook to fire twice, resulting in two duplicate messages whenever something is committed. With my testing, this doesn't seem to happen using this action.

Does this still happen? I don't see duplicate commits in the current commit stream channel using the default discord formatted webhook.

[Amazinite]

Does this still happen? I don't see duplicate commits in the current commit stream channel using the default discord formatted webhook.

Because people are deleting all the duplicates.

[MasterOfGrey]

and it would not be susceptible to users editing the formatting but not knowing about the Discord integration.

This particular point is entirely a non-issue.

[Hecter94]

Yes, but it isn't ideal, as we lose a lot of information on the handoff.

Even if it isn't ideal, could we use it for now until we get a better implementation? It sounds like this is something we could start using immediately while we work on getting James set up to handle the webhook payloads, which from what you and MCO are saying, sounds like it will be a decent amount of work and probably take a while to get going.

Realistically, for the "Commit Stream" that this would be doing, the only information your general user will want is just what the commit changed in a human-readable format which is information most readily available via the commit title.

In the future, if we want to start doing more with this, I agree with your suggestion to send the payload directly to James, but for now, I'd like to get something in place even if it's not ideal since it will work for what we need, and work much better than the default discord-formatted webhook.

Edit:
I suppose the alternative would be to continue using the default-discord formatted webhook and then configure James to automatically delete the duplicates.
I think that would be a reasonable compromise as well, and I expect it wouldn't be too difficult to set James up to do that.
The main disadvantage there is that commit titles would continue getting cut off, but it's usually still possible to get a good idea of what the commit changed via what it does show.

[MCOfficer]

Even if it isn't ideal, could we use it for now until we get a better implementation? It sounds like this is something we could start using immediately while we work on getting James set up to handle the webhook payloads, which from what you and MCO are saying, sounds like it will be a decent amount of work and probably take a while to get going.

^This. Let's not overengineer a perfectly expandable solution that we may never truly need. If the need for role pings or more faceted notifications comes up, by all means let's explore all possible solutions - but let's also not force ourselves into a "perfect is the enemy of good" situation over such a mundane thing as notifications. This is not complicated code you will have to maintain for years, it's a yaml file that can be deleted any time.

[quyykk]

Agreed with MCO. If we ever update James we can simply remove this yml file.

[tehhowch]

I'm curious how one tags a Discord user/group in the given message, and if Ilshidur/action-discord#82 is going to be applicable. (Both those questions don't block this PR.)

[tehhowch]

As an example, both the author name and the commit message may contain :, so a consumer is not guaranteed to be able to parse the information from just the Discord message.

I personally think there should be a link to view the commit, too, but YMMV.

[Hecter94]

Changed this to commit message committed by author.

I am open to additional feedback on this and would like to hear what kind of formatting others might want and whether a link is something we want.

[MasterOfGrey]

I'm curious how one tags a Discord user/group in the given message

It's just a text code string pushed out in the message intended to be printed:
<@userid> for a user
<@&roleid> for a role

[tehhowch]

QA time: what, specifically, would be posted to Discord by this proposed workflow if the commit pushed is e.g. aa6cccb

In other words, you're posting whatever is in the head_commit object's message field. How much stuff might that be? 100 characters? 1000? If it's more than 2000 Discord is going to reject the request, but I suspect you're going to want to cap it much more quickly than that.

[tehhowch]

Ideally the workflow should not run, rather than just skipping a step in a single job. Is that possible?

[MCOfficer]

Haven't found anything. See for example actions/runner#520

[Hecter94]

Yeah, I looked and didn't find any way to stop the "workflow" itself from running, since I'm pretty sure it has to run in order for the secret to be loaded into an environment variable in order to be used in a conditional.
Though if anyone does know of a way, I'd be more than happy to add it.

[tehhowch]

The previous format of "author before commit" is more natural to read. My previous comment was just calling out the possible ambiguity involved when parsing the messages produced by this job; This format also has ambiguity, since committed by is not a reserved construct that is unable to appear in the commit message.

[Hecter94]

QA time: what, specifically, would be posted to Discord by this proposed workflow if the commit pushed is, e.g. aa6cccb

In other words, you're posting whatever is in the head_commit object's message field. How much stuff might that be? 100 characters? 1000? If it's more than 2000, Discord will reject the request, but I suspect you'll want to cap it much more quickly.

I tried it out, looks like it came through like this, which may be a bit longer than we'd like.
Ideally, I would limit it to just the commit title, e.g. fix(internal): Various fixes to the code found by MSVC though looking at https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#push I'm not sure that's possible.
If that isn't possible, capping it at around 200 characters would likely be best since that should give enough information to understand the commit without having long messages in the Discord channel.

[tehhowch]

I tried it out, looks like it came through like this, which may be a bit longer than we'd like.
Ideally, I would limit it to just the commit title, e.g. fix(internal): Various fixes to the code found by MSVC though looking at https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#push I'm not sure that's possible.
If that isn't possible, capping it at around 200 characters would likely be best since that should give enough information to understand the commit without having long messages in the Discord channel.

You should consider the commit title to be whatever is before the first 2 newline characters (either 2 \n or 2 \r\n, depending on platform), as the commit message format follows the title with a blank line.

That screenshot also suggests you need to add handling for link references (#xxxx) which may or may not be in the title, depending on if the commit is a push or merge commit. Much more often than not, they will be present.

[Hecter94]

Some of the suggestions brought up are going to require sending the webhook as a discord embed JSON object rather than a string like it is now.
This is going to require redoing how I'm building the webhook and I'll need to sanitize the inputs as well so that the JSON doesn't break and do some string manipulation to get the commit title and commit message separately.

Given that's going to be a fairly substantial edit to what I already have, I think it's best that I put this to draft for now while I work on doing that.

[Hecter94]

I don't see myself getting to this any time soon, so I'm going to close it so as to not clutter up our PRs