Skip to content

feat(engine): add commit-level evaluation primitives with ordered multi-commit support#6402

Open
sachin9058 wants to merge 3 commits intomindersec:mainfrom
sachin9058:feat/commit-evaluation-primitives-v2
Open

feat(engine): add commit-level evaluation primitives with ordered multi-commit support#6402
sachin9058 wants to merge 3 commits intomindersec:mainfrom
sachin9058:feat/commit-evaluation-primitives-v2

Conversation

@sachin9058
Copy link
Copy Markdown
Contributor

Summary

This PR introduces commit-level evaluation primitives for pull request analysis, enabling evaluation of individual commits rather than only the final state (HEAD).

Currently, Minder evaluates only the resulting state of a PR, which can allow intermediate commits containing issues (e.g., vulnerable dependencies or invalid metadata) to be introduced into the repository history. This change provides a flexible foundation to support evaluation across all commits in a PR.

Key changes include:

  • Added CommitPolicy interface for defining commit-level checks
  • Implemented CommitEvaluator for evaluating commits against one or more policies
  • Added EvaluateAll to evaluate all commits in a PR while preserving order
  • Introduced CommitResult to capture structured per-commit evaluation results
  • Added HasPolicyFailures helper for aggregation of results
  • Retained HasFailures as a deprecated alias for backward compatibility
  • Added ConventionalCommitPolicy as an example policy for commit message validation

This PR focuses on providing reusable evaluation primitives rather than enforcing specific policies. It enables building policies such as:

  • Checking intermediate commit contents
  • Enforcing commit message formats
  • Validating commit metadata (e.g., signatures)

This builds on previous work:

Fixes #2176


Testing

The changes were tested using unit tests covering:

  • Evaluation with no policies configured

  • Evaluation with single and multiple policies

  • Multi-commit evaluation using EvaluateAll

  • Preservation of commit order in results

  • Aggregation behavior using HasPolicyFailures

  • Edge cases such as empty commit lists

  • Validation of commit message formats using ConventionalCommitPolicy, including:

    • Valid conventional commits
    • Scopes with slashes (e.g., api/v1)
    • Uppercase scopes
    • Invalid commit messages

All tests were run using:

go test ./...

No additional configuration is required.


Note

This PR is a recreation of the previous one after the branch was accidentally reset during cleanup. The implementation itself remains unchanged.

@sachin9058 sachin9058 requested a review from a team as a code owner April 22, 2026 22:21
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 60.391% (+0.05%) from 60.337% — sachin9058:feat/commit-evaluation-primitives-v2 into mindersec:main

Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to focus on extensible policies for per-commit evaluation, rather than policies which are hard-coded into the Minder engine. (e.g. the osv, frizbee, trusty evaluators are probably an anti-pattern that are artifacts of early implementation before we fully understood the problem)

@sachin9058
Copy link
Copy Markdown
Contributor Author

@evankanderson

The intention with this PR is to provide generic commit-level evaluation primitives (e.g., CommitPolicy, CommitEvaluator, and EvaluateAll) rather than embedding specific policies into the engine.

The example ConventionalCommitPolicy was mainly added to demonstrate how policies can be implemented on top of these primitives, not as something tightly coupled to the engine itself.

I agree that keeping the system extensible and avoiding hard-coded evaluators in the engine is the right direction. Happy to adjust the structure further if you’d prefer moving example policies out or changing how they’re exposed.

@sachin9058
Copy link
Copy Markdown
Contributor Author

@evankanderson Thanks, that makes a lot of sense.

I’ve updated the PR to keep the engine focused on the evaluation primitives and removed the concrete policy from it. The evaluator now just works with the CommitPolicy interface, and I switched the tests to use simple mock policies instead.

The idea is to keep this layer generic and let actual policies live outside the engine so they can evolve independently.

Happy to adjust further if you think something should be structured differently.

@evankanderson
Copy link
Copy Markdown
Member

See my suggestion in #2176 (comment) -- I think you want to write out an example eval section of a rule for implementing one of these policies before building the primitives, to make sure that you're building evaluation primitives that will support the policies you want -- for example, you could either build an API which exposes "get me the added and removed commits between branch A and branch B" and "get me metadata for commit X" or an API which exposes "get me the metadata for commits added to branch B not in branch A".

I don't know which is better, but writing down an example or three of using each API will give you a good idea about the limitations (expressiveness, repetition, control, etc) of each API.

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.

Not all commits being added to main are checked during PR

3 participants