FINERACT-2462: Add github action to enforce one commit per user in a PR by edk12564 · Pull Request #5437 · apache/fineract

edk12564 · 2026-02-03T03:51:19Z

Description

JIRA: FINERACT-2462

Implemented a Github Action that runs on PR request changes, opens, and closes.

The Github Action checks for:

Does each user have a single PR associated with their Github account for that PR?
Does each user have a valid git email configured to match their Github email?

The action on failure is to comment in the PR thread with the type of error, as well as show in the error’s logs.

The feature allows for multiple users per PR, but only 1 commit per user.

Edge Cases and Behavior

Test Cases I have tried:

If git email does not match any emails registered for the github account, the check will fail. This is because they will have null values for author.id. The author fields are required to differentiate users, and make sure they are not bots.
If there are multiple commits associated with an account, the check will fail.
If all goes well, there will be a successful check and a pass.
When a user has a misconfigured git account, and pushes their code, they will get the error that one of the committers have their git email misconfigured. If they fix and recommit without squashing, this issue will still persist since the old commit is still there. Need to recommend a squash as well to remove the commit history. Fixed now.

Edge Cases covered in code:

If a PR is made with only bots, like from another Github action, the test should pass.
If Github API is down, the test should fail.
If there are both bots and users, only the users commits should be looked at.

Failure Path (misconfigured git email):

Failure Path (multiple commits per user):

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

Write the commit message as per our guidelines
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow our coding conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

Aman-Mittal · 2026-02-03T04:26:21Z

.github/workflows/pr-one-commit-per-user-check.yml

+     - name: Comment on PR
+       if: failure()
+       env:
+         GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}


Is token is necessary in this?

From my understanding that Github token isn't the same one we use to authenticate our accounts. It's a separate token created per workflow for using the Github API calls in your actions. I believe it is required to access .author.id.

More on this here:
https://docs.github.com/en/actions/tutorials/authenticate-with-github_token

Seems Ok, Can you also check the workflow of stale.yml, i really like the way it checks prs. This will not run without workflow approval and stale is a cron based job.

I'll take a look now!

I did a bit of research, and I agree with your view. The Stale workflow has a lot of merits, and I will look into adding to what we have. To summarize though, this is what I found.

1 - The current implementation needs permissions for it to run properly. Stale.yml uses the main repo's context, which allows it to run without manual approval.

On this note, I found something we could use. If we use pull_request_target instead of pull_request, we could run our workflows with base repo permissions. I am reading about it here: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target.

The main issue for this seems to be that this raises security concerns about running code without prior approval. But I think this might be fine considering we are only looking at the commit repository, PR number, and userIds. We don't actually execute any foreign code. Let me know what you think on this one!

2 - I also see what you mean about running a cronjob. I think this is a good idea as well. However, if we want to run on a cronjob to handle older PRs, the issue is that we probably shouldn't spam comments in case of users abandoning their PR. I think we should implement something like checking the most recent comment, or maybe we won't produce warnings if the no one has committed in the past day or two? I will take a look and let you know what our options are here.

What do you think on those points?

@Aman-Mittal Hey Aman! Thanks for posting that point. It was a direction I hadn't thought of. I agree that if the direction of the project is to sign all our commits to prove contribution, it's a concern that we can only sign one person. I also brought up the point of Github not recognizing contribution unless it's a commit or co-authorship as well. Let's see what everyone says!

I dont think we need cron job. It will be executed alongside all other checks. When something got changed, it will be re-run. I see no issue here.

I think we are fine to have multiple commits if it came from different authors. The check should catch and handle the situation where:

Commit by Author A -> So far good Commit by Author A -> Nope, it's a duplication!

or

Commit by Author A -> So far good Commit by Author B -> So far good Commit by Author A -> Nope, it's a duplication!

Hi adam! Really sorry about this but I made a post in the dev mail list before I read this comment. That's completely my bad.

To summarize the post, I wrote on a potential improvement to One Commit per PR that might allow us to keep full credit for everyone involved. We can add co-contributors to Co-Authored-By and also the trailer they performed work in, like Designed-By or Tested-By. This gives them full contribution in Github's eyes without having multiple users in a commit. But the tradeoff is still that each person cannot sign their commit, like they can in One Commit per User as Aman mentioned.

Would you still like One Commit Per User here given the new information? I will say that the implementation works either way!

I have read your recommendations on the mail list. Thank you for sharing there. Hopefully others will chime in. I am just 1 voice from the many ;)

Sounds good to me! Thank you sir 👍

adamsaghy · 2026-02-11T10:44:13Z

87 actionable tasks: 36 executed, 37 from cache, 14 up-to-date
Execution failed for task ':rat'.
> A failure occurred while executing org.nosphere.apache.rat.RatWork
   > Apache Rat audit failure - 1 unapproved license
     	See file:///home/runner/work/fineract/fineract/build/reports/rat/index.html

edk12564 · 2026-02-11T16:08:19Z

87 actionable tasks: 36 executed, 37 from cache, 14 up-to-date
Execution failed for task ':rat'.
> A failure occurred while executing org.nosphere.apache.rat.RatWork
   > Apache Rat audit failure - 1 unapproved license
     	See file:///home/runner/work/fineract/fineract/build/reports/rat/index.html

Sorry about that. I'll fix this now!

- added commit check - accounted for edge case where git and github emails don't match - accounted for edge case where wrong git account hasn't squashed DesignedBy: Edward Kang and Aman Mittal ReviewedBy: Aman Mittal ReportedBy: Aman Mittal

adamsaghy · 2026-02-18T15:28:39Z

.github/workflows/pr-one-commit-per-user-check.yml

+name: Fineract PR One Commit Per User Check
+
+
+on:


This should be below no?

on: pull_request: types: [opened, synchronize, reopened]

Hi adam! Thank you for the review. I took a look again, and realized I was responding to the wrong thing. I thought you were referring to the the types, not the pull_request_target.

I think pull_request_target was used here because it allows us to run this check in the main repo's context, which me and Aman figured was okay considered no foreign code is actually being executed. This would allow us to run the check without needing someone to approve the test first.

More on this here: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target. Do you think this is okay, or is it more policy to make sure everything is approved first?

My concern is this newly added github actions were not executed on this PR which means it is targeting the wrong thing, no?

I see what you mean with the target! To test this out, I did some investigating on my local fork where I submitted a PR into my own fork's develop branch.

My setup is: the develop branch has pr-one-commit-per-user.yml and the PR branch does not. I first tested with multiple commits at once, and it properly failed. I also tested the target by squashing my commits in the PR, and the test passed. From the results, it seems to properly target the PR branch, but runs the pull_request_target scripts that live in the develop branch. Some pictures below.

no script in PR:

failed test:

success test:

I think based on this, this one particular PR will not have checks applied. However, if it is merged into the develop branch, any future PR will still be checked, and target the actual PR branch, and not the apache/fineract repo. However, the problem of running without workflow permissions isn't a big issue, so I think we can definitely go either way. Please let me know what you think!

Running this workflow on the target branch is incorrect. You should use the one i shared, imho:

on: pull_request: types: [opened, synchronize, reopened]

edk12564 force-pushed the FINERACT-2462 branch from ee6574f to 65d1cba Compare February 3, 2026 04:05

Aman-Mittal reviewed Feb 3, 2026

View reviewed changes

edk12564 force-pushed the FINERACT-2462 branch from 65d1cba to eb3491a Compare February 3, 2026 04:41

Aman-Mittal mentioned this pull request Feb 6, 2026

FINERACT-2462: Add GitHub Action to validate one commit per author in PRs #5453

Closed

6 tasks

edk12564 force-pushed the FINERACT-2462 branch 3 times, most recently from 705c0a9 to c45fec9 Compare February 6, 2026 10:44

edk12564 changed the title ~~FINERACT-2436: Add github action to enforce one commit per user in a PR~~ FINERACT-2462: Add github action to enforce one commit per user in a PR Feb 11, 2026

edk12564 force-pushed the FINERACT-2462 branch from c45fec9 to d101ab7 Compare February 11, 2026 16:34

edk12564 force-pushed the FINERACT-2462 branch from d101ab7 to 9c8fa98 Compare February 11, 2026 22:19

adamsaghy reviewed Feb 18, 2026

View reviewed changes

Conversation

edk12564 commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Edge Cases and Behavior

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edk12564 Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edk12564 Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edk12564 Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsaghy commented Feb 11, 2026

Uh oh!

edk12564 commented Feb 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edk12564 Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edk12564 Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

edk12564 commented Feb 3, 2026 •

edited

Loading

edk12564 Feb 3, 2026 •

edited

Loading

edk12564 Feb 9, 2026 •

edited

Loading

edk12564 Feb 18, 2026 •

edited

Loading

edk12564 Feb 18, 2026 •

edited

Loading

edk12564 Feb 19, 2026 •

edited

Loading