Skip to content

fix: Skip merge commits in DCO sign-off check#1202

Open
rwalworth wants to merge 7 commits intomainfrom
01199-beginner-fix-dco-bot-to-skip-merge-commits
Open

fix: Skip merge commits in DCO sign-off check#1202
rwalworth wants to merge 7 commits intomainfrom
01199-beginner-fix-dco-bot-to-skip-merge-commits

Conversation

@rwalworth
Copy link
Contributor

@rwalworth rwalworth commented Mar 4, 2026

Summary

This PR fixes the DCO (Developer Certificate of Origin) bot to skip merge commits when
validating sign-off lines. Merge commits are auto-generated by Git and do not contain
original work, so DCO sign-off should not be required for them.

Fixes #1199.


Motivation

In PR #1198, the bot flagged commit b41b844 (Merge branch 'main' into feat/getAccountInfo-jsonRpc#669) as missing DCO sign-off. This is a false positive - merge commits have more than one parent and are generated by Git, not authored by a contributor. The DCO check should exempt them.


Changes

isMergeCommit Helper

Added a new isMergeCommit(commit) function in checks.js that detects merge commits
by checking whether the parents array has more than one entry (the standard GitHub API
representation of a merge commit):

function isMergeCommit(commit) {
  return Array.isArray(commit?.parents) && commit.parents.length > 1;
}

checkDCO Update

Updated the checkDCO loop to skip merge commits before checking for a Signed-off-by:
line. The log message now reports how many merge commits were skipped for transparency:

DCO check: 1/1 passed (1 merge commit(s) skipped)

Only the DCO check is affected — the GPG signature check and merge conflict check are
unchanged, as the issue requires.

Test Utilities

Added a commitMerge(sha, message) factory to test-utils.js that creates a merge
commit object (two parents, no DCO sign-off, verified GPG) for use in tests.

Unit Tests (6 + 3 new)

Added to test-checks.js:

Test Verifies
isMergeCommit: commit with two parents returns true Positive detection
isMergeCommit: commit with one parent returns false Regular commit
isMergeCommit: commit with no parents field returns false Missing field
isMergeCommit: null commit returns false Null safety
isMergeCommit: undefined commit returns false Undefined safety
isMergeCommit: commit with empty parents array returns false Empty array
checkDCO: merge commit without sign-off is skipped (passes) Merge-only PR passes
checkDCO: merge commit skipped, regular commits still checked Mixed: failure still caught
checkDCO: all regular commits pass with merge commit present Mixed: all pass

Integration Tests (2 new)

Added to test-on-pr-update-bot.js:

Test Trigger Verifies
[sync] Merge commit without DCO sign-off is skipped synchronize All checks pass, label swaps to needs-review
[edit] Merge commit without DCO sign-off is skipped edited All checks pass, comment shows all green

Testing

node .github/scripts/tests/test-checks.js              # ✅ 58 tests pass
node .github/scripts/tests/test-on-pr-update-bot.js    # ✅ 13 sync + 14 integration pass
node .github/scripts/tests/test-on-pr-open-bot.js      # ✅ All pass (unchanged)
node .github/scripts/tests/test-comments.js             # ✅ All pass (unchanged)
node .github/scripts/tests/test-api.js                  # ✅ All pass (unchanged)
node .github/scripts/tests/test-assign-bot.js           # ✅ All pass (unchanged)

Test Plan:

  • All existing tests still pass (no regressions)
  • isMergeCommit correctly identifies merge vs. non-merge commits
  • checkDCO skips merge commits and still flags unsigned regular commits
  • Integration tests confirm merge commits don't cause false positives end-to-end

Files Changed Summary

Category Files Notes
Source .github/scripts/helpers/checks.js isMergeCommit helper + checkDCO update
Test Utilities .github/scripts/tests/test-utils.js commitMerge factory
Unit Tests .github/scripts/tests/test-checks.js 9 new tests
Integration Tests .github/scripts/tests/test-on-pr-update-bot.js 2 new scenarios

Breaking Changes

None. The only behavioral change is that merge commits are no longer flagged by the
DCO check. All other checks (GPG, merge conflict, issue link) are unaffected.

Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth rwalworth self-assigned this Mar 4, 2026
@rwalworth rwalworth requested review from a team as code owners March 4, 2026 17:56
@rwalworth rwalworth added the status: blocked Work is stalled due to external dependency or required decision label Mar 4, 2026
@rwalworth rwalworth requested a review from jeromy-cannon March 4, 2026 17:56
@rwalworth rwalworth linked an issue Mar 4, 2026 that may be closed by this pull request
22 tasks
@github-actions github-actions bot added the status: needs review The pull request is ready for maintainer review label Mar 4, 2026
@rwalworth rwalworth removed the status: needs review The pull request is ready for maintainer review label Mar 4, 2026
@github-actions
Copy link

github-actions bot commented Mar 4, 2026

Hi, this is DCO Bot! 👋

I noticed some commits on this PR are missing the required DCO sign-off. Here are the ones that need attention:

  • e8b4fcd Merge branch 'main' into 01199-beginner-fix-dco-bot-to-skip-merge-commits

Please add a line Signed-off-by: Your Name <your.email@example.com> to each commit (e.g. git commit -s). For more info, see the Signing Guide.

@rwalworth rwalworth added status: needs review The pull request is ready for maintainer review and removed status: blocked Work is stalled due to external dependency or required decision labels Mar 5, 2026
…mits

Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

Hi @rwalworth 👋, thank you for submitting this PR!
I'm your PR Helper Bot and I'll be keeping track of your PR's
status to help you get it approved and merged.

This comment will stay updated as you make changes.
Here's where things stand:


PR Checks

DCO Sign-off -- The following commits are missing the required DCO sign-off:

  • e8b4fcd Merge branch 'main' into 01199-beginner-fix-dco-bot-to-skip-merge-commits

Please add Signed-off-by: Your Name <email> to each commit (e.g. git commit -s). See the Signing Guide.


GPG Signature -- All commits have verified GPG signatures.


Merge Conflicts -- No merge conflicts detected.


Issue Link -- Linked to #1199 (assigned to you).


All checks must pass before this PR can be reviewed.

@github-actions github-actions bot added status: needs revision A pull request that requires changes before merge and removed status: needs review The pull request is ready for maintainer review labels Mar 5, 2026
@rwalworth rwalworth added status: needs review The pull request is ready for maintainer review and removed status: needs revision A pull request that requires changes before merge labels Mar 5, 2026
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@github-actions github-actions bot added status: needs revision A pull request that requires changes before merge and removed status: needs review The pull request is ready for maintainer review labels Mar 5, 2026
@rwalworth rwalworth added status: needs review The pull request is ready for maintainer review and removed status: needs revision A pull request that requires changes before merge labels Mar 5, 2026
@github-actions github-actions bot added status: needs revision A pull request that requires changes before merge and removed status: needs review The pull request is ready for maintainer review labels Mar 5, 2026
@rwalworth rwalworth added status: needs review The pull request is ready for maintainer review and removed status: needs revision A pull request that requires changes before merge labels Mar 5, 2026
@github-actions github-actions bot added status: needs revision A pull request that requires changes before merge and removed status: needs review The pull request is ready for maintainer review labels Mar 5, 2026
@rwalworth rwalworth added status: needs review The pull request is ready for maintainer review and removed status: needs revision A pull request that requires changes before merge labels Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs review The pull request is ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Beginner]: Fix DCO Bot to Skip Merge Commits

1 participant