Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 7, 2025

Validate that at least one check is enabled

Problem

The action could be misconfigured with both check-commits and check-pull-request set to false, which would result in the action not performing any validation. This creates a confusing user experience where the action runs successfully but doesn't actually check anything.

Solution

Added early validation to fail the action if both checks are disabled. The validation:

  • Runs immediately after input retrieval, before any processing begins
  • Provides a clear error message indicating at least one check must be enabled
  • Returns early to prevent unnecessary processing

Changes

src/index.js

  • Added validation check after input retrieval (lines 42-48)
  • Fails with error message if both check-commits and check-pull-request are false
  • Returns early to prevent unnecessary processing

tests/index.test.js

  • Added 4 comprehensive test cases covering all input combinations:
    • Both checks disabled → fails with error ✅
    • Only check-commits enabled → passes ✅
    • Only check-pull-request enabled → passes ✅
    • Both checks enabled → passes ✅

Testing

All tests passing:

  • ✅ 55 JavaScript tests (51 original + 4 new)
  • ✅ 29 bash tests
  • ✅ Build successful (npm run package)
  • ✅ Linting passes

Fixes the issue where users could misconfigure the action with both checks disabled.

Original prompt

This section details on the original issue you should resolve

<issue_title>Validate that commit check or pr check is enabled</issue_title>
<issue_description>If both check-commits and check-pull-request is false, fail the action (at least 1 should be enabled) </issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #65

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add validation for commit and PR check configurations Validate that at least one check is enabled (check-commits or check-pull-request) Oct 7, 2025
Copilot AI requested a review from joshjohanning October 7, 2025 16:46
Copilot finished work on behalf of joshjohanning October 7, 2025 16:46
@joshjohanning
Copy link
Owner

@copilot can you rebase and re-make these changes? I changed my action pretty significantly since originally requesting this (now native node action and not composite action with shell).

Copy link
Contributor Author

Copilot AI commented Nov 7, 2025

@copilot can you rebase and re-make these changes? I changed my action pretty significantly since originally requesting this (now native node action and not composite action with shell).

I've rebased and re-implemented the validation for the new native Node.js action structure.

The changes are now in commit bf7f3bb:

  • Added validation in src/index.js (lines 42-48) to check that at least one of check-commits or check-pull-request is enabled
  • Added 4 comprehensive test cases in __tests__/index.test.js covering all input combinations
  • All 55 JavaScript tests + 29 bash tests passing

Note: The branch needs to be force-pushed to update the PR since it was rebased from the old composite action structure onto main. I've prepared the commit locally but don't have push permissions. You may need to pull the changes or I can create a new PR if needed.

Copilot finished work on behalf of joshjohanning November 7, 2025 20:43
@joshjohanning joshjohanning force-pushed the copilot/validate-checks-configuration branch from 9c0f050 to c6ec8d1 Compare November 7, 2025 21:15
@joshjohanning
Copy link
Owner

@copilot I had to manually git fetch origin main && git reset --hard origin/main to get the latest in this working branch. Now you have to re-implement the original requirement.

Add validation to ensure at least one of 'check-commits' or 'check-pull-request' is enabled.
If both are set to false, the action will fail with a clear error message.

Changes:
- Add validation check in src/index.js (lines 42-48)
- Add 4 comprehensive test cases in __tests__/index.test.js
- All 55 JavaScript tests + 29 bash tests passing

Co-authored-by: joshjohanning <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 7, 2025

@copilot I had to manually git fetch origin main && git reset --hard origin/main to get the latest in this working branch. Now you have to re-implement the original requirement.

I've re-implemented the validation check in commit 2a9db9a.

Changes:

  • Added validation in src/index.js (lines 42-48) to ensure at least one of check-commits or check-pull-request is enabled
  • Added 4 comprehensive test cases in __tests__/index.test.js covering all input combinations
  • All 55 JavaScript tests + 29 bash tests passing
  • Build and linting successful

Copilot finished work on behalf of joshjohanning November 7, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate that commit check or pr check is enabled

2 participants