feat(github): add required_signatures branch-protection tools (#178)#180
Conversation
GitHub exposes commit-signature enforcement on a separate endpoint
(.../protection/required_signatures), so it could not be set through
github_update_branch_protection. Closes that gap by adding a dedicated
enable/disable/get trio, mirroring the existing
github_*_vulnerability_alerts pattern.
New tools:
- github_get_required_signatures(repo_owner, repo_name, branch)
- github_enable_required_signatures(repo_owner, repo_name, branch)
- github_disable_required_signatures(repo_owner, repo_name, branch)
Endpoint: PUT/DELETE/GET /repos/{owner}/{repo}/branches/{branch}/protection/required_signatures
- security.py: three async functions using github_client_context().
PUT returns 200+JSON (not 204 like vulnerability_alerts PUT);
DELETE returns 204; GET returns 200 with {enabled, url} or 404
when branch protection is absent.
- models.py: three Pydantic models with (repo_owner, repo_name, branch).
- registry_github.py: ToolDefinition trio added after vulnerability_alerts.
- api.py untouched (wildcard re-export of security.py covers new funcs).
- README.md tool counts: 52 -> 55 and 51/22 -> 54/25.
- tests/unit/github/test_github_required_signatures.py: 7 tests via
AsyncMock+patch(github_client_context) covering get (enabled/disabled/
404), enable (200/404), disable (204/404).
Closes #178
|
Claude finished @MementoRC's task —— View job PR Review: GitHub Required Signatures Branch Protection ToolsI've completed my comprehensive review of this PR. Here's my assessment: Todo List:
Overall Assessment: ✅ EXCELLENT IMPLEMENTATIONThis is a very well-implemented PR that follows established patterns and maintains high code quality. The implementation is clean, consistent, and comprehensive. Code Quality & Best Practices ✅Excellent adherence to existing patterns:
Strong model design:
Security Considerations ✅Appropriate security implementation:
Performance Considerations ✅Efficient implementation:
Test Coverage & Quality ✅Comprehensive test suite (
Test Cases Coverage:
Architecture & Integration ✅Seamless integration with existing codebase:
Minor ObservationsSlight inconsistency noted (not a bug):
No issues found:
Recommendation: ✅ APPROVEThis PR is ready for merge. It demonstrates excellent engineering practices:
The implementation correctly fills the gap mentioned in the issue description by providing dedicated tools for GitHub's separate required_signatures endpoint, maintaining consistency with the existing |
Summary
GitHub exposes commit-signature enforcement on a separate REST endpoint (
.../protection/required_signatures), so it cannot be set throughgithub_update_branch_protection's payload. Closes that gap by adding a dedicated enable/disable/get trio, mirroring the existinggithub_*_vulnerability_alertspattern.New tools:
github_get_required_signatures(repo_owner, repo_name, branch)github_enable_required_signatures(repo_owner, repo_name, branch)github_disable_required_signatures(repo_owner, repo_name, branch)Endpoint:
PUT|DELETE|GET /repos/{owner}/{repo}/branches/{branch}/protection/required_signaturesChanges
github/security.py— three async functions using the existinggithub_client_context(). The PUT returns 200+JSON (unlike the vulnerability_alerts PUT which returns 204), DELETE returns 204, GET returns 200 with{enabled, url}or 404 when branch protection is absent.github/models.py— three Pydantic models, each(repo_owner, repo_name, branch).lean/registry_github.py—ToolDefinitiontrio added after the vulnerability_alerts block, usingmodel_json_schema().github/api.py— untouched; the existingfrom .security import *wildcard re-export picks up the new functions automatically.README.md— lean interface counts: 52 → 55 (total), 51 → 54 / 22 → 25 (GitHub bucket).tests/unit/github/test_github_required_signatures.py— 7 tests viaAsyncMock+patch(github_client_context).Test plan
pixi run -e quality ruff check <5 changed files>— cleanpixi run -e quality ruff format --check <5 changed files>— clean (pre-existing format debt on unrelated files is not introduced here)pixi run -e quality pytest tests/unit/github/test_github_required_signatures.py -v— 7/7 passpixi run -e quality pytest tests/unit/github/ -v— 266/266 pass, no regressionsTest coverage
test_get_returns_enabled_when_status_200_and_enabled_trueenabled:truebody →✅ Required signatures: enabledtest_get_returns_disabled_when_status_200_and_enabled_falseenabled:falsebody →❌ Required signatures: disabledtest_get_returns_not_found_when_status_404❌ Branch protection or branch not found:test_enable_returns_success_when_status_200test_enable_returns_error_when_status_404test_disable_returns_success_when_status_204test_disable_returns_error_when_status_404All tests verify the correct URL
/repos/foo/bar/branches/main/protection/required_signaturesis hit on the right HTTP verb.Why dedicated tools, not a parameter on update_branch_protection
The issue notes the dedicated-tools approach is “probably the cleaner fit” because:
github_enable_vulnerability_alerts/github_enable_automated_security_fixes.github_update_branch_protectionfrom silently making a second API call hidden inside its payload.Closes #178