Skip to content

Expose push safety validation#9

Merged
shenxianpeng merged 5 commits into
mainfrom
update-py-version
May 18, 2026
Merged

Expose push safety validation#9
shenxianpeng merged 5 commits into
mainfrom
update-py-version

Conversation

@shenxianpeng
Copy link
Copy Markdown
Member

@shenxianpeng shenxianpeng commented May 18, 2026

Summary

  • bump commit-check dependency to >=2.7.0 so MCP can use the upstream no_force_push rule
  • add validate_push_safety and optional include_push repository-state validation
  • document the new MCP tool and cover push safety behavior in tests

closes #8

Validation

  • uv run pytest
  • git diff --check
  • uv run python smoke check for commit-check 2.7.0 PUSH_RULES/no_force_push

Summary by CodeRabbit

Release Notes

  • New Features

    • Added push-safety validation tool to validate push operations
    • Extended repository state validation with optional push checks
  • Documentation

    • Updated documentation to cover push-safety validation features

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

📝 Walkthrough

Walkthrough

This PR adds push-safety validation to the MCP server through a new validate_push_safety tool, extends existing validation with an optional include_push flag, documents the feature in the README, adds comprehensive test coverage, and updates Dependabot configuration.

Changes

Push Safety Validation Feature

Layer / File(s) Summary
Push validation internal implementation and rule imports
src/commit_check_mcp/server.py
Server imports PUSH_RULES catalog and updates instructions to mention push-safety validation. New _validate_push validator merges config, enforces push.allow_force_push = False, and runs the no_force_push rule with context derived from whether push_refs is provided.
Push safety MCP tool registration
src/commit_check_mcp/server.py
Added validate_push_safety MCP tool that normalizes push_refs, resolves repo and config paths, and delegates validation to _validate_push.
Repository state and rule discovery integration
src/commit_check_mcp/server.py
Extended validate_repository_state with include_push flag (default False) to append push checks when enabled. Updated describe_validation_rules to include push-rule identifiers in supported_checks by combining all three rule catalogs.
Documentation updates
README.md
Added validate_push_safety to features and tool-usage sections with parameter documentation. Included example showing pre-push hook ref metadata usage and sample JSON payload. Added "Repository-Aware Validation" bullet for push-safety checks. Updated repository-wide example to demonstrate include_push: true.
Push safety validation test coverage
tests/test_server.py
Added tests for validate_push_safety tool and _validate_push internal validator. Extended existing validate_repository_state tests with include_push flag. Updated "combines requested checks" test to mock _validate_push and assert no_force_push appears in aggregated checks. Added assertion to "describes validation rules" test confirming no_force_push in supported_checks.

Dependabot Configuration Update

Layer / File(s) Summary
Dependabot pip pattern configuration
.github/dependabot.yml
Adjusted the YAML structure/placement of the wildcard pattern entry under the pip ecosystem's dependency groups.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

A rabbit hops in with joy,
Push safety checks now employed!
Force-push we forbid,
Upstream we consider hid,
The validation layers deployed! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Expose push safety validation' directly matches the PR's main objective of adding push-safety validation features.
Linked Issues check ✅ Passed The PR fulfills issue #8's requirement to document push-force validation in README.md and implements the validate_push_safety tool with full documentation.
Out of Scope Changes check ✅ Passed All changes are scoped to implementing push-safety validation: dependabot.yml fix, documentation updates, new MCP tool, extended validation, and comprehensive tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch update-py-version

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread .github/dependabot.yml Outdated
@shenxianpeng shenxianpeng added the enhancement New feature or request label May 18, 2026
@shenxianpeng shenxianpeng changed the title [codex] expose push safety validation Expose push safety validation May 18, 2026
@shenxianpeng shenxianpeng marked this pull request as ready for review May 18, 2026 23:54
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 18, 2026
@shenxianpeng shenxianpeng removed the documentation Improvements or additions to documentation label May 18, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.02%. Comparing base (6c7136b) to head (b34038c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
+ Coverage   62.94%   66.02%   +3.08%     
==========================================
  Files           2        2              
  Lines         197      209      +12     
==========================================
+ Hits          124      138      +14     
+ Misses         73       71       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shenxianpeng shenxianpeng merged commit 880eadf into main May 18, 2026
6 of 7 checks passed
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commit_check_mcp/server.py`:
- Around line 406-409: The current normalization sets normalized_push_refs =
push_refs.strip() which turns whitespace-only strings into "" and causes
_validate_push to treat them as explicit refs; change the normalization so that
after stripping any string that becomes empty is coerced to None (e.g., use
push_refs.strip() or None) so normalized_push_refs is None for
empty/whitespace-only input before calling _validate_push; update the assignment
that creates normalized_push_refs (and keep using _normalize_repo_path and the
_validate_push call) so downstream logic uses upstream fallback correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 348d62d1-e17a-4137-9fd4-1940d2325108

📥 Commits

Reviewing files that changed from the base of the PR and between 6c7136b and b34038c.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • .github/dependabot.yml
  • README.md
  • src/commit_check_mcp/server.py
  • tests/test_server.py

Comment on lines +406 to +409
normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
normalized_repo_path = _normalize_repo_path(repo_path)
return _validate_push(
normalized_push_refs,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject whitespace-only push_refs input to avoid fallback misrouting.

On Line 406, whitespace-only input is normalized to "" and forwarded. That makes _validate_push treat it as explicit refs (push_upstream_fallback=False) instead of upstream fallback. Please reject empty strings (or coerce them to None) after normalization.

Suggested fix
 def validate_push_safety(
@@
 ) -> dict[str, Any]:
     """Validate that a push is not a force push."""
     normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
+    if isinstance(push_refs, str) and not normalized_push_refs:
+        raise ValueError("push_refs cannot be empty when provided")
     normalized_repo_path = _normalize_repo_path(repo_path)
     return _validate_push(
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
normalized_repo_path = _normalize_repo_path(repo_path)
return _validate_push(
normalized_push_refs,
normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
if isinstance(push_refs, str) and not normalized_push_refs:
raise ValueError("push_refs cannot be empty when provided")
normalized_repo_path = _normalize_repo_path(repo_path)
return _validate_push(
normalized_push_refs,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commit_check_mcp/server.py` around lines 406 - 409, The current
normalization sets normalized_push_refs = push_refs.strip() which turns
whitespace-only strings into "" and causes _validate_push to treat them as
explicit refs; change the normalization so that after stripping any string that
becomes empty is coerced to None (e.g., use push_refs.strip() or None) so
normalized_push_refs is None for empty/whitespace-only input before calling
_validate_push; update the assignment that creates normalized_push_refs (and
keep using _normalize_repo_path and the _validate_push call) so downstream logic
uses upstream fallback correctly.

@shenxianpeng shenxianpeng deleted the update-py-version branch May 18, 2026 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add validate_force_push tool to README.md

2 participants