fix: add /retest support for security checks

[myakove]

Problem

/retest security-committer-identity and /retest security-suspicious-paths return "No ... configured for this repository" even when security checks are enabled and working.

Root Cause

_current_pull_request_supported_retest didn't include security check names, and run_retests didn't have security check functions in its dispatch map.

Fix

• Add security checks to _current_pull_request_supported_retest when enabled
• Add security runners to run_retests dispatch map (with lambda wrappers since security methods don't take pull_request param)

Closes #1122

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 fix: /retest does not support security-committer-identity and security-suspicious-paths

1. Untyped dict in tests ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new test functions annotate minimal_hook_data as dict without type parameters, which
violates the repo's strict mypy settings (disallow_any_generics = true) and can cause CI/mypy
failures.

Code

webhook_server/tests/test_github_api.py[R1323-1360]

+    def test_supported_retest_includes_security_checks(
+        self, minimal_hook_data: dict, minimal_headers: Headers, logger: Mock
+    ) -> None:
+        """Test that security checks are included in supported retests when enabled."""
+        with patch("webhook_server.libs.github_api.Config") as mock_config:
+            mock_config.return_value.repository = True
+            mock_config.return_value.repository_local_data.return_value = {}
+
+            with patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") as mock_get_api:
+                mock_get_api.return_value = (Mock(), "token", "apiuser")
+
+                with patch("webhook_server.libs.github_api.get_github_repo_api") as mock_get_repo_api:
+                    mock_get_repo_api.return_value = Mock()
+
+                    with patch("webhook_server.libs.github_api.get_repository_github_app_api") as mock_get_app_api:
+                        mock_get_app_api.return_value = Mock()
+
+                        with patch("webhook_server.utils.helpers.get_repository_color_for_log_prefix") as mock_color:
+                            mock_color.return_value = "test-repo"
+
+                            gh = GithubWebhook(minimal_hook_data, minimal_headers, logger)
+
+                            gh.security_committer_identity_check = True
+                            gh.security_suspicious_paths = ["Dockerfile", ".github/"]
+                            gh.tox = False
+                            gh.build_and_push_container = False
+                            gh.pypi = False
+                            gh.pre_commit = False
+                            gh.conventional_title = False
+                            gh.custom_check_runs = []
+
+                            result = gh._current_pull_request_supported_retest
+                            assert SECURITY_COMMITTER_IDENTITY_STR in result
+                            assert SECURITY_SUSPICIOUS_PATHS_STR in result
+
+    def test_supported_retest_excludes_disabled_security_checks(
+        self, minimal_hook_data: dict, minimal_headers: Headers, logger: Mock
+    ) -> None:

Evidence

pyproject.toml enables strict mypy with disallow_any_generics = true, and the added tests use
minimal_hook_data: dict (missing type parameters) in their signatures.

CLAUDE.md: Provide Complete Type Hints Compatible with Strict mypy
webhook_server/tests/test_github_api.py[1323-1325]
webhook_server/tests/test_github_api.py[1358-1360]
pyproject.toml[26-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New tests use `minimal_hook_data: dict` (unparameterized generic), which violates strict mypy (`disallow_any_generics=true`) and may fail type checking.

## Issue Context
The repository enforces strict mypy settings in `pyproject.toml`, including `disallow_any_generics = true`, so unparameterized generics like `dict` are not allowed.

## Fix Focus Areas
- webhook_server/tests/test_github_api.py[1323-1361]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. run_retests lambda kwarg mismatch ✓ Resolved 📎 Requirement gap ≡ Correctness

Description

run_retests() invokes each registered runner as runner(pull_request=pull_request), but the newly
added security retest lambdas are defined to accept _pull_request instead, so Python will raise a
TypeError for an unexpected pull_request keyword and break /retest security-committer-identity
and /retest security-suspicious-paths at runtime.

Code

webhook_server/libs/handlers/runner_handler.py[R1976-1979]

+        _retests_to_func_map[SECURITY_COMMITTER_IDENTITY_STR] = lambda _pull_request: (
+            self.run_security_committer_identity()
+        )
+        _retests_to_func_map[SECURITY_SUSPICIOUS_PATHS_STR] = lambda _pull_request: self.run_security_suspicious_paths()

Evidence

The dispatcher loop in RunnerHandler.run_retests() schedules all retest runners by calling them
with the keyword argument pull_request=..., while the PR registers the security retest handlers as
lambda wrappers whose only parameter name is _pull_request (used because the underlying security
methods don’t accept a pull request). Since Python binds keyword arguments by name, those lambdas
will not accept the pull_request keyword and will fail at runtime with `TypeError: got an
unexpected keyword argument 'pull_request'`, violating the requirement that the security retest
names be dispatched successfully.

Add security check runners to retest dispatch map
webhook_server/libs/handlers/runner_handler.py[1974-1990]
webhook_server/libs/handlers/runner_handler.py[1947-2001]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RunnerHandler.run_retests()` calls each retest runner with `runner(pull_request=...)`, but the new security retest runners are lambda wrappers whose parameter name doesn’t match (`_pull_request`), causing a runtime `TypeError` and preventing the security retests from executing.

## Issue Context
- The dispatch loop invokes runners via `runner(pull_request=pull_request)`.
- Security retests were added as lambda wrappers because the underlying security methods don’t accept a `pull_request` parameter.
- The wrappers therefore must accept the `pull_request` keyword (even if unused) or accept `**kwargs`, so the existing call-site remains valid and `/retest security-committer-identity` and `/retest security-suspicious-paths` dispatch correctly.
- One explicit option is to use keyword-only parameters to make intent clear, e.g. `lambda *, pull_request: self.run_security_committer_identity()` (ignoring `pull_request`).

## Fix Focus Areas
- webhook_server/libs/handlers/runner_handler.py[1974-1990]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Security retests lack coverage ✓ Resolved 📎 Requirement gap ☼ Reliability

Description

The new security-retest support and dispatch code paths are not exercised by existing retest-related
tests, increasing regression risk. Add tests that enable security checks and assert both
supported-retest inclusion and run_retests dispatch for security-committer-identity and
security-suspicious-paths.

Code

webhook_server/libs/github_api.py[R1418-1423]

+        # Add security checks when enabled
+        if self.security_committer_identity_check:
+            current_pull_request_supported_retest.append(SECURITY_COMMITTER_IDENTITY_STR)
+
+        if self.security_suspicious_paths:
+            current_pull_request_supported_retest.append(SECURITY_SUSPICIOUS_PATHS_STR)

Evidence

PR Compliance ID 3 requires tests that (1) assert security checks appear in the supported retest
list when enabled and (2) validate run_retests dispatch for both security checks. The new
support-list logic is present, but existing tests shown do not enable/verify security retests,
leaving the new branches untested.

Add tests covering retesting of security checks
webhook_server/libs/github_api.py[1413-1425]
webhook_server/tests/test_github_api.py[1286-1321]
webhook_server/tests/test_issue_comment_handler.py[33-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New `/retest` support for `security-committer-identity` and `security-suspicious-paths` was added, but current tests don’t cover the new security-enabled branches or verify dispatch/execution.

## Issue Context
There is a test for `_current_pull_request_supported_retest`, but it enables only non-security checks. There are also issue-comment handler tests that set `current_pull_request_supported_retest` without security checks.

## Fix Focus Areas
- webhook_server/libs/github_api.py[1418-1423]
- webhook_server/libs/handlers/runner_handler.py[1974-1990]
- webhook_server/tests/test_github_api.py[1286-1321]
- webhook_server/tests/test_issue_comment_handler.py[33-55]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[myakove-bot]

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

• Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
• Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
• Issue Creation: Disabled for this repository
• Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
• Branch Labeling: Branch-specific labels are applied to track the target branch
• Auto-verification: Auto-verified users have their PRs automatically marked as verified
• Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

• /wip - Mark PR as work in progress (adds WIP: prefix to title)
• /wip cancel - Remove work in progress status
• /hold - Block PR merging (approvers only)
• /hold cancel - Unblock PR merging
• /verified - Mark PR as verified
• /verified cancel - Remove verification status
• /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
• /regenerate-welcome - Regenerate this welcome message
• /security-override - Set security check runs to pass (maintainers only)
• /security-override cancel - Re-run security checks

Review & Approval

• /lgtm - Approve changes (looks good to me)
• /approve - Approve PR (approvers only)
• /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
• /assign-reviewers - Assign reviewers based on OWNERS file
• /assign-reviewer @username - Assign specific reviewer
• /check-can-merge - Check if PR meets merge requirements

Testing & Validation

• /retest tox - Run Python test suite with tox
• /retest build-container - Rebuild and test container image
• /retest python-module-install - Test Python package installation
• /retest pre-commit - Run pre-commit hooks and checks
• /retest conventional-title - Validate commit message format
• /retest all - Run all available tests

Container Operations

• /build-and-push-container - Build and push container image (tagged with PR number)
  • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

• /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
  • Multiple branches: /cherry-pick branch1 branch2 branch3
• /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

• /rebase - Rebase this PR branch onto its base branch

Label Management

• /<label-name> - Add a label to the PR
• /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

1. Approval: /approve from at least one approver
2. LGTM Count: Minimum 1 /lgtm from reviewers
3. Status Checks: All required status checks must pass
4. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
5. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

• myakove
• rnetser

Reviewers:

• myakove
• rnetser

Available Labels

• hold
• verified
• wip
• lgtm
• approve
• automerge

AI Features

• Conventional Title: Mode: fix (claude/claude-opus-4-6[1m])
• Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6[1m])
• Test Oracle: Triggers: approved (claude/claude-opus-4-6[1m]); /test-oracle can be used anytime

Security Checks

• Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
• Committer Identity Check: Verifies last committer matches PR author
• Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

• WIP Status: Use /wip when your PR is not ready for review
• Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
• Cherry-picking: Cherry-pick labels are processed when the PR is merged
• Container Builds: Container images are automatically tagged with the PR number
• Permission Levels: Some commands require approver permissions
• Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

[qodo-code-review]

PR Summary by Qodo

Fix /retest support for security checks
🐞 Bug fix 🕐 10-20 Minutes

Description

• Include enabled security checks in the repository’s supported /retest list.
• Add security check runners to the run_retests dispatch map.
• Prevent false “No … configured for this repository” responses for security retests.

Diagram

graph TD
  A["IssueCommentHandler: /retest"] --> B["GithubWebhook: supported retests"] --> C["supported_retests list"] --> D["RunnerHandler.run_retests"] --> E["retest dispatch map"] --> F["Security runners"] --> G["CheckRunHandler updates"] --> H{{"GitHub Checks API"}}

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Unify runner signatures (accept optional pull_request)

• ➕ Removes the need for lambda wrappers in the dispatch map
• ➕ Keeps dispatch values as direct method references (more consistent/debuggable)
• ➖ Requires changing security runner method signatures (broader API surface change)
• ➖ Potentially touches more call sites and tests than necessary for this bug fix

2. Make run_retests support mixed signatures (introspection/adapter)

• ➕ Allows dispatching both PR-based and non-PR-based runners without wrappers
• ➕ Keeps runner implementations unchanged
• ➖ Adds complexity to run_retests (signature inspection/branching)
• ➖ Harder to reason about and type-check than explicit wrappers

Recommendation: The PR’s approach is appropriate for a targeted bug fix: it wires security checks into the supported retest list and dispatch map with minimal behavioral change elsewhere. The lambda wrappers are a small, explicit adapter that avoids refactoring runner method signatures or adding introspection logic in the scheduler.

Files changed (2) +16 / -0

Bug fix (2) +16 / -0

github_api.pyExpose enabled security checks as supported '/retest' targets +9/-0

Expose enabled security checks as supported '/retest' targets

• Imports the security check name constants and appends them to '_current_pull_request_supported_retest' when the corresponding security features are enabled. This ensures '/retest security-committer-identity' and '/retest security-suspicious-paths' are recognized as configured for the repo.

webhook_server/libs/github_api.py

runner_handler.pyDispatch '/retest' for security checks to security runners +7/-0

Dispatch '/retest' for security checks to security runners

• Extends 'run_retests()'’s check-name-to-runner map to include the two security checks. Uses small lambda adapters because the security runner methods don’t accept a 'pull_request' argument, allowing the existing scheduling logic to invoke them via the same interface.

webhook_server/libs/handlers/runner_handler.py

[qodo-code-review]

Code review by qodo was updated up to the latest commit 64f7230

[myakove]

@qodo-code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/libs/handlers/runner_handler.py:1976 (qodo requirement gap) — run_retests lambda kwarg mismatch

Addressed: Fixed — changed lambda _pull_request to lambda **_kwargs to accept keyword args correctly.

webhook_server/libs/github_api.py:1418 (qodo requirement gap) — Security retests lack coverage

Addressed: Fixed — added tests for security retest list inclusion and exclusion.

[qodo-code-review]

Code review by qodo was updated up to the latest commit b0d7305

[myakove]

@qodo-code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/tests/test_github_api.py:1323 (qodo rule violation) — Untyped dict in tests

Addressed: Fixed — changed minimal_hook_data: dict to dict[str, Any] in both test methods.

webhook_server/libs/github_api.py:1418 (qodo requirement gap) — Security retests lack coverage

Skipped: Already addressed in commit 64f7230 — added test_supported_retest_includes_security_checks and test_supported_retest_excludes_disabled_security_checks.

[myakove-bot]

New container for ghcr.io/myk-org/github-webhook-server:latest published