From 9048834f09ee75d9eb603e28ce5b1fe7fb547bab Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 09:11:15 +0300 Subject: [PATCH 01/25] feat: support user-defined custom commands in PR welcome message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add custom-commands configuration option (global and per-repo) that renders user-defined commands in a new Custom Commands section of the PR welcome message. These are documentation-only — the server displays them but does not process them, allowing external bots/tools to handle them. Closes #1132 Signed-off-by: rnetser Assisted-by: Claude --- examples/config.yaml | 9 ++ webhook_server/config/schema.yaml | 26 +++++ .../libs/handlers/pull_request_handler.py | 18 +++ .../test_prepare_retest_welcome_comment.py | 107 +++++++++++++++++- 4 files changed, 159 insertions(+), 1 deletion(-) diff --git a/examples/config.yaml b/examples/config.yaml index cd77362f..78fd13df 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -314,3 +314,12 @@ repositories: # - ".github/workflows/" # - ".github/actions/" # committer-identity-check: true + + # Custom commands to display in PR welcome message (documentation-only) + # These commands are shown in the welcome comment but not processed by the server. + # External bots or tools handle them independently. + # custom-commands: + # - name: deploy-staging + # description: Deploy this PR to the staging environment + # - name: run-e2e + # description: Trigger end-to-end test suite against this PR diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 815b9954..63985be5 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -716,3 +716,29 @@ properties: - name - command additionalProperties: false + custom-commands: + type: array + description: | + Custom commands to display in the PR welcome message. + These are documentation-only - the server renders them in the welcome + message but does NOT process them. External bots or tools handle them. + + Examples: + - name: deploy-staging + description: Deploy this PR to the staging environment + - name: run-e2e + description: Trigger end-to-end test suite against this PR + items: + type: object + properties: + name: + type: string + pattern: "^[a-zA-Z0-9_-]+$" + description: Command name (without the leading slash) + description: + type: string + description: Human-readable description of what the command does + required: + - name + - description + additionalProperties: false diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index a511123a..aca6313c 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -497,6 +497,7 @@ def _prepare_welcome_comment(self) -> str: {self._prepare_retest_welcome_comment} {self._prepare_container_operations_welcome_section}\ {self._prepare_cherry_pick_section}\ +{self._prepare_custom_commands_welcome_section}\ #### Label Management * `/` - Add a label to the PR @@ -842,6 +843,23 @@ def _prepare_cherry_pick_section(self) -> str: """ return "\n#### Branch Management\n* `/rebase` - Rebase this PR branch onto its base branch\n" + @property + def _prepare_custom_commands_welcome_section(self) -> str: + """Prepare the Custom Commands section for the welcome comment. + + Renders user-defined custom commands from configuration. + These are documentation-only - the server does not process them. + """ + custom_commands: list[dict[str, str]] = self.github_webhook.config.get_value("custom-commands", []) + if not custom_commands: + return "" + + lines: list[str] = ["\n#### Custom Commands"] + for cmd in custom_commands: + lines.append(f"* `/{cmd['name']}` - {cmd['description']}") + + return "\n".join(lines) + "\n" + async def label_all_opened_pull_requests_merge_state_after_merged(self) -> None: """ Labels pull requests based on their mergeable state. diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index c47a9da7..9aa035fb 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -1,7 +1,7 @@ from __future__ import annotations import re -from typing import TYPE_CHECKING, TypedDict, Unpack +from typing import TYPE_CHECKING, Any, TypedDict, Unpack import pytest @@ -348,3 +348,108 @@ def test_retest_content_between_testing_header_and_container_section( # Should contain retest commands assert "/retest tox" in section_between, "Retest tox command should be in Testing & Validation section" + + +class TestCustomCommandsWelcomeSection: + """Tests for the custom commands welcome section feature. + + Verifies that user-defined custom commands from configuration + are rendered correctly in the welcome message. + """ + + def _create_handler( + self, + process_github_webhook: GithubWebhook, + owners_file_handler: OwnersFileHandler, + custom_commands: list[dict[str, str]] | None = None, + ) -> PullRequestHandler: + process_github_webhook.tox = False + process_github_webhook.build_and_push_container = False + process_github_webhook.pypi = False + process_github_webhook.pre_commit = False + process_github_webhook.conventional_title = False + process_github_webhook.parent_committer = "test-user" + process_github_webhook.auto_verified_and_merged_users = [] + process_github_webhook.create_issue_for_new_pr = False + process_github_webhook.issue_url_for_welcome_msg = "https://github.com/test/repo/issues/1" + process_github_webhook.minimum_lgtm = 1 + process_github_webhook.pull_request = None + + owners_file_handler.all_pull_request_approvers = ["approver1"] + owners_file_handler.all_pull_request_reviewers = ["reviewer1"] + + # Mock config.get_value for custom-commands + original_get_value = process_github_webhook.config.get_value + + def mock_get_value(key: str, default: Any = None, **kwargs: Any) -> Any: + if key == "custom-commands": + return custom_commands if custom_commands is not None else [] + return original_get_value(key, default, **kwargs) + + process_github_webhook.config.get_value = mock_get_value # type: ignore[assignment] + + return PullRequestHandler(github_webhook=process_github_webhook, owners_file_handler=owners_file_handler) + + def test_custom_commands_not_configured( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """When no custom commands are configured, welcome message should not contain the section.""" + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=[]) + welcome_msg = handler._prepare_welcome_comment() + assert "#### Custom Commands" not in welcome_msg + + def test_custom_commands_configured( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """When custom commands are configured, welcome message should contain the section and command text.""" + commands = [ + {"name": "deploy-staging", "description": "Deploy this PR to staging"}, + {"name": "run-e2e", "description": "Run e2e tests"}, + ] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + assert "#### Custom Commands" in welcome_msg + assert "/deploy-staging" in welcome_msg + assert "Deploy this PR to staging" in welcome_msg + assert "/run-e2e" in welcome_msg + assert "Run e2e tests" in welcome_msg + + def test_custom_commands_formatting( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """Each custom command should be formatted as: * `/name` - description.""" + commands = [ + {"name": "deploy-staging", "description": "Deploy this PR to staging"}, + {"name": "run-e2e", "description": "Run e2e tests"}, + ] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + assert "* `/deploy-staging` - Deploy this PR to staging" in welcome_msg + assert "* `/run-e2e` - Run e2e tests" in welcome_msg + + def test_custom_commands_header_on_own_line( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """The Custom Commands header should be preceded by a newline.""" + commands = [{"name": "deploy-staging", "description": "Deploy this PR to staging"}] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + assert "\n#### Custom Commands\n" in welcome_msg, "#### Custom Commands header should be on its own line" + + def test_custom_commands_appears_before_label_management( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """The Custom Commands section should appear before the Label Management section.""" + commands = [{"name": "deploy-staging", "description": "Deploy this PR to staging"}] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + custom_pos = welcome_msg.find("#### Custom Commands") + label_pos = welcome_msg.find("#### Label Management") + + assert custom_pos != -1, "Custom Commands section should be present" + assert label_pos != -1, "Label Management section should be present" + assert custom_pos < label_pos, "Custom Commands section should appear before Label Management section" From 9ffaf4f238409909b6c79bcaa5adf43703fbee61 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 11:22:03 +0300 Subject: [PATCH 02/25] fix: address Qodo review findings for custom-commands Add custom-commands to root-level schema for global config support. Add runtime validation with warning logs for malformed entries. Add schema validation tests for custom-commands configuration. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/config/schema.yaml | 20 +++++ .../libs/handlers/pull_request_handler.py | 22 ++++- webhook_server/tests/test_config_schema.py | 82 +++++++++++++++++++ 3 files changed, 120 insertions(+), 4 deletions(-) diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 63985be5..35ee9118 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -253,6 +253,26 @@ properties: $ref: '#/$defs/ai-features' security-checks: $ref: '#/$defs/security-checks' + custom-commands: + type: array + description: | + Custom commands to display in the PR welcome message (global default). + These are documentation-only - the server renders them in the welcome + message but does NOT process them. External bots or tools handle them. + items: + type: object + properties: + name: + type: string + pattern: "^[a-zA-Z0-9_-]+$" + description: Command name (without the leading slash) + description: + type: string + description: Human-readable description of what the command does + required: + - name + - description + additionalProperties: false labels: type: object description: Configure which labels are enabled and their colors diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index aca6313c..37b225b9 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -849,14 +849,28 @@ def _prepare_custom_commands_welcome_section(self) -> str: Renders user-defined custom commands from configuration. These are documentation-only - the server does not process them. + Invalid entries are skipped with a warning log. """ - custom_commands: list[dict[str, str]] = self.github_webhook.config.get_value("custom-commands", []) - if not custom_commands: + raw_commands: object = self.github_webhook.config.get_value("custom-commands", []) + if not isinstance(raw_commands, list) or not raw_commands: return "" lines: list[str] = ["\n#### Custom Commands"] - for cmd in custom_commands: - lines.append(f"* `/{cmd['name']}` - {cmd['description']}") + for cmd in raw_commands: + if not isinstance(cmd, dict): + self.logger.warning(f"{self.log_prefix} Skipping invalid custom-command entry: not a dict") + continue + + name = cmd.get("name") + description = cmd.get("description") + if not isinstance(name, str) or not isinstance(description, str) or not name or not description: + self.logger.warning(f"{self.log_prefix} Skipping custom-command with missing name or description") + continue + + lines.append(f"* `/{name}` - {description}") + + if len(lines) == 1: + return "" return "\n".join(lines) + "\n" diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index 6a600257..6cacea60 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -5,6 +5,7 @@ import pytest import yaml +from jsonschema import ValidationError, validate from webhook_server.libs.config import Config @@ -1006,3 +1007,84 @@ def test_ai_features_resolve_cherry_pick_conflicts_with_ai_repository_level( } finally: shutil.rmtree(temp_dir) + + +class TestCustomCommandsSchema: + """Tests for custom-commands schema validation.""" + + def test_schema_allows_global_custom_commands(self) -> None: + """Test that schema.yaml defines custom-commands at root level.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + assert "custom-commands" in schema["properties"] + assert schema["properties"]["custom-commands"]["type"] == "array" + + def test_schema_allows_per_repo_custom_commands(self) -> None: + """Test that schema.yaml defines custom-commands in per-repo properties.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + repo_props = schema["properties"]["repositories"]["additionalProperties"]["properties"] + assert "custom-commands" in repo_props + assert repo_props["custom-commands"]["type"] == "array" + + def test_custom_commands_valid_config(self) -> None: + """Test that valid custom-commands config passes schema validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], + "custom-commands": [ + {"name": "deploy-staging", "description": "Deploy to staging"}, + {"name": "run-e2e", "description": "Run e2e tests"}, + ], + } + validate(instance=config, schema=schema) + + def test_custom_commands_rejects_missing_name(self) -> None: + """Test that custom-commands without name fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], + "custom-commands": [ + {"description": "Missing name field"}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) + + def test_custom_commands_rejects_invalid_name_pattern(self) -> None: + """Test that custom-commands with invalid name pattern fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], + "custom-commands": [ + {"name": "invalid name with spaces", "description": "Bad name"}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) + + def test_custom_commands_rejects_extra_properties(self) -> None: + """Test that custom-commands with extra properties fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], + "custom-commands": [ + {"name": "test", "description": "Test", "extra": "not allowed"}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) From 5e69b62eec4e46c235dbe383fbf9614be2b2577a Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 11:36:05 +0300 Subject: [PATCH 03/25] fix: address cycle 2 Qodo findings Add minItems constraint, jsonschema test dependency, token allowlist annotations, and runtime name pattern validation with description sanitization. Signed-off-by: Ruth Netser Assisted-by: Claude --- pyproject.toml | 2 +- webhook_server/config/schema.yaml | 2 ++ webhook_server/libs/handlers/pull_request_handler.py | 8 +++++++- webhook_server/tests/test_config_schema.py | 8 ++++---- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3aaac411..08b3ed18 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -109,4 +109,4 @@ dev = [ "types-pyyaml>=6.0.12.20250516", "types-requests>=2.32.4.20250611", ] -tests = ["psutil>=7.0.0", "pytest-asyncio>=0.26.0", "pytest-xdist>=3.7.0"] +tests = ["jsonschema>=4.0.0", "psutil>=7.0.0", "pytest-asyncio>=0.26.0", "pytest-xdist>=3.7.0"] diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 35ee9118..304b29b4 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -255,6 +255,7 @@ properties: $ref: '#/$defs/security-checks' custom-commands: type: array + minItems: 1 description: | Custom commands to display in the PR welcome message (global default). These are documentation-only - the server renders them in the welcome @@ -738,6 +739,7 @@ properties: additionalProperties: false custom-commands: type: array + minItems: 1 description: | Custom commands to display in the PR welcome message. These are documentation-only - the server renders them in the welcome diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index 37b225b9..ab38163f 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -2,6 +2,7 @@ import asyncio import hashlib +import re import shlex import traceback from collections.abc import Coroutine @@ -867,7 +868,12 @@ def _prepare_custom_commands_welcome_section(self) -> str: self.logger.warning(f"{self.log_prefix} Skipping custom-command with missing name or description") continue - lines.append(f"* `/{name}` - {description}") + if not re.fullmatch(r"[a-zA-Z0-9_-]+", name): + self.logger.warning(f"{self.log_prefix} Skipping custom-command with invalid name: {name!r}") + continue + + sanitized_description = description.replace("\n", " ").replace("\r", " ") + lines.append(f"* `/{name}` - {sanitized_description}") if len(lines) == 1: return "" diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index 6cacea60..d5fd8205 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -1036,7 +1036,7 @@ def test_custom_commands_valid_config(self) -> None: schema = yaml.safe_load(f) config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], + "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ {"name": "deploy-staging", "description": "Deploy to staging"}, {"name": "run-e2e", "description": "Run e2e tests"}, @@ -1051,7 +1051,7 @@ def test_custom_commands_rejects_missing_name(self) -> None: schema = yaml.safe_load(f) config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], + "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ {"description": "Missing name field"}, ], @@ -1066,7 +1066,7 @@ def test_custom_commands_rejects_invalid_name_pattern(self) -> None: schema = yaml.safe_load(f) config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], + "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ {"name": "invalid name with spaces", "description": "Bad name"}, ], @@ -1081,7 +1081,7 @@ def test_custom_commands_rejects_extra_properties(self) -> None: schema = yaml.safe_load(f) config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], + "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ {"name": "test", "description": "Test", "extra": "not allowed"}, ], From 0bd51f3d0fe1e6e3288b85e3df8433f86ca37556 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 11:56:29 +0300 Subject: [PATCH 04/25] fix: allow per-repo custom-commands empty list override Remove minItems from per-repo custom-commands schema to allow repositories to set custom-commands: [] to disable global defaults. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/config/schema.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 304b29b4..70ad282c 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -739,7 +739,6 @@ properties: additionalProperties: false custom-commands: type: array - minItems: 1 description: | Custom commands to display in the PR welcome message. These are documentation-only - the server renders them in the welcome From dbd57854c5da0c1ce36e3ff8c90b63e5b483afb6 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 12:18:42 +0300 Subject: [PATCH 05/25] fix: load custom-commands via repo-local config Load custom-commands through _repo_data_from_config with extra_dict so .github-webhook-server.yaml per-repo overrides are respected. Read from self.github_webhook.custom_commands attribute in handler. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 7 +++++++ webhook_server/libs/handlers/pull_request_handler.py | 6 +++--- .../tests/test_prepare_retest_welcome_comment.py | 12 ++---------- webhook_server/tests/test_pull_request_handler.py | 1 + 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index deef20fa..50d624e6 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -941,6 +941,13 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: ) self.custom_check_runs: list[dict[str, Any]] = self._validate_custom_check_runs(raw_custom_checks) + raw_custom_commands = self.config.get_value( + value="custom-commands", return_on_none=[], extra_dict=repository_config + ) + self.custom_commands: list[dict[str, str]] = ( + raw_custom_commands if isinstance(raw_custom_commands, list) else [] + ) + _auto_users = self.config.get_value( value="auto-verified-and-merged-users", return_on_none=[], extra_dict=repository_config ) diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index ab38163f..f4693382 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -852,12 +852,12 @@ def _prepare_custom_commands_welcome_section(self) -> str: These are documentation-only - the server does not process them. Invalid entries are skipped with a warning log. """ - raw_commands: object = self.github_webhook.config.get_value("custom-commands", []) - if not isinstance(raw_commands, list) or not raw_commands: + custom_commands: list[dict[str, str]] = self.github_webhook.custom_commands + if not custom_commands: return "" lines: list[str] = ["\n#### Custom Commands"] - for cmd in raw_commands: + for cmd in custom_commands: if not isinstance(cmd, dict): self.logger.warning(f"{self.log_prefix} Skipping invalid custom-command entry: not a dict") continue diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index 9aa035fb..5ee2af47 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -1,7 +1,7 @@ from __future__ import annotations import re -from typing import TYPE_CHECKING, Any, TypedDict, Unpack +from typing import TYPE_CHECKING, TypedDict, Unpack import pytest @@ -378,15 +378,7 @@ def _create_handler( owners_file_handler.all_pull_request_approvers = ["approver1"] owners_file_handler.all_pull_request_reviewers = ["reviewer1"] - # Mock config.get_value for custom-commands - original_get_value = process_github_webhook.config.get_value - - def mock_get_value(key: str, default: Any = None, **kwargs: Any) -> Any: - if key == "custom-commands": - return custom_commands if custom_commands is not None else [] - return original_get_value(key, default, **kwargs) - - process_github_webhook.config.get_value = mock_get_value # type: ignore[assignment] + process_github_webhook.custom_commands = custom_commands if custom_commands is not None else [] return PullRequestHandler(github_webhook=process_github_webhook, owners_file_handler=owners_file_handler) diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index 4a7642a1..fe831f1c 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -92,6 +92,7 @@ def mock_github_webhook(self) -> Mock: mock_webhook.ctx = None mock_webhook.enabled_labels = None # Default: all labels enabled mock_webhook.custom_check_runs = [] + mock_webhook.custom_commands = [] mock_webhook.ai_features = None mock_webhook.required_conversation_resolution = False mock_webhook.security_suspicious_paths = [] From 49093587866aabe81b495e90d5b31a7db58d3555 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 12:28:13 +0300 Subject: [PATCH 06/25] fix: add warning log for non-list custom-commands config Log a warning when custom-commands config value is not a list, matching the pattern used by _validate_custom_check_runs. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 50d624e6..4bdf530a 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -944,9 +944,16 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: raw_custom_commands = self.config.get_value( value="custom-commands", return_on_none=[], extra_dict=repository_config ) - self.custom_commands: list[dict[str, str]] = ( - raw_custom_commands if isinstance(raw_custom_commands, list) else [] - ) + if not isinstance(raw_custom_commands, list): + if raw_custom_commands is not None: + prefix = getattr(self, "log_prefix", "") + self.logger.warning( + f"{prefix} custom-commands config is not a list " + f"(got {type(raw_custom_commands).__name__}), skipping" + ) + self.custom_commands: list[dict[str, str]] = [] + else: + self.custom_commands = raw_custom_commands _auto_users = self.config.get_value( value="auto-verified-and-merged-users", return_on_none=[], extra_dict=repository_config From c67389b8b8197575ee1ee66996a6903c2eb76161 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 20:50:29 +0300 Subject: [PATCH 07/25] test: add coverage tests for push_handler, config, ai_cli, and pr_review_handler Increase test coverage from 89.47% to 90.02% by adding tests for: - PushHandler: context tracking, pypi upload, container build paths - Config: label validation, root_data error handling, repo local data - AI CLI: sidecar available/unavailable paths - PullRequestReviewHandler: context step tracking Co-authored-by: PI (claude-opus-4-6) Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/tests/test_ai_cli.py | 63 ++++++- webhook_server/tests/test_config.py | 158 +++++++++++++++- .../tests/test_pull_request_review_handler.py | 32 ++++ webhook_server/tests/test_push_handler.py | 169 ++++++++++++++++++ 4 files changed, 413 insertions(+), 9 deletions(-) diff --git a/webhook_server/tests/test_ai_cli.py b/webhook_server/tests/test_ai_cli.py index 7eafd44f..bdcef3b7 100644 --- a/webhook_server/tests/test_ai_cli.py +++ b/webhook_server/tests/test_ai_cli.py @@ -2,7 +2,11 @@ from __future__ import annotations -from webhook_server.libs.ai_cli import get_ai_config +from unittest.mock import AsyncMock, patch + +import pytest + +from webhook_server.libs.ai_cli import AIResult, call_ai, get_ai_config class TestGetAiConfig: @@ -23,3 +27,60 @@ def test_get_ai_config_partial_missing_model(self) -> None: def test_get_ai_config_partial_missing_provider(self) -> None: assert get_ai_config({"ai-model": "sonnet"}) is None + + +class TestCallAi: + """Test suite for call_ai function.""" + + @pytest.mark.asyncio + async def test_call_ai_sidecar_unavailable(self) -> None: + """Test call_ai returns error when sidecar is unavailable.""" + with patch( + "webhook_server.libs.ai_cli.check_sidecar_available", + new_callable=AsyncMock, + return_value=(False, "connection refused"), + ): + result = await call_ai( + prompt="test", + ai_provider="claude", + ai_model="sonnet", + cwd="/tmp", + ) + assert result.success is False + assert "Pi-sidecar unavailable" in result.error + assert "connection refused" in result.error + + @pytest.mark.asyncio + async def test_call_ai_sidecar_available(self) -> None: + """Test call_ai delegates to call_ai_once when sidecar is available.""" + expected = AIResult(success=True, text="hello", error="") + with patch( + "webhook_server.libs.ai_cli.check_sidecar_available", + new_callable=AsyncMock, + return_value=(True, "ok"), + ): + with patch( + "webhook_server.libs.ai_cli.call_ai_once", + new_callable=AsyncMock, + return_value=expected, + ) as mock_call: + result = await call_ai( + prompt="test", + ai_provider="claude", + ai_model="sonnet", + cwd="/tmp", + timeout_minutes=5, + system_prompt="be helpful", + ) + assert result.success is True + assert result.text == "hello" + mock_call.assert_called_once_with( + prompt="test", + ai_provider="claude", + ai_model="sonnet", + cwd="/tmp", + ai_call_timeout=5, + system_prompt="be helpful", + tools=None, + custom_tools=None, + ) diff --git a/webhook_server/tests/test_config.py b/webhook_server/tests/test_config.py index e310ea14..e32e3232 100644 --- a/webhook_server/tests/test_config.py +++ b/webhook_server/tests/test_config.py @@ -221,9 +221,8 @@ def test_repository_local_data_list_result(self, temp_config_dir: str, monkeypat assert result == {"local-setting": "value"} - @patch("webhook_server.utils.helpers.get_github_repo_api") def test_repository_local_data_file_not_found( - self, mock_get_repo_api: Mock, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch ) -> None: """Test repository_local_data method when config file is not found.""" monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) @@ -231,27 +230,25 @@ def test_repository_local_data_file_not_found( # Mock repository that raises UnknownObjectException mock_repo = Mock() mock_repo.get_contents.side_effect = UnknownObjectException(404, "Not found") - mock_get_repo_api.return_value = mock_repo config = Config(repository="test-repo") mock_github_api = Mock() + mock_github_api.get_repo.return_value = mock_repo result = config.repository_local_data(mock_github_api, "org/test-repo") assert result == {} - @patch("webhook_server.utils.helpers.get_github_repo_api") def test_repository_local_data_exception_handling( - self, mock_get_repo_api: Mock, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch ) -> None: """Test repository_local_data method with exception handling.""" monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) - # Mock repository that raises an exception - mock_get_repo_api.side_effect = Exception("API Error") - config = Config(repository="test-repo") mock_github_api = Mock() + # Make get_repo raise a generic exception + mock_github_api.get_repo.side_effect = Exception("API Error") result = config.repository_local_data(mock_github_api, "org/test-repo") @@ -400,3 +397,148 @@ def test_get_value_priority_order(self, temp_config_dir: str, monkeypatch: pytes # Test priority: repository_data should win over root_data result = config.get_value("test-key") assert result == "repo-value" + + def test_validate_labels_config_global_invalid_labels( + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test validate_labels_config raises ValueError for invalid global enabled-labels.""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + config_file = os.path.join(temp_config_dir, "config.yaml") + config_data = { + "github-app-id": 123456, + "github-tokens": ["token1"], + "webhook-ip": "http://localhost:5000", + "repositories": {"test-repo": {"name": "org/test-repo"}}, + "labels": {"enabled-labels": ["totally-bogus-category"]}, + } + with open(config_file, "w") as f: + yaml.dump(config_data, f) + + with pytest.raises(ValueError, match="Invalid label categories in enabled-labels"): + Config() + + def test_validate_labels_config_repo_level_invalid_labels( + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test validate_labels_config raises ValueError for invalid repo-level enabled-labels.""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + config_file = os.path.join(temp_config_dir, "config.yaml") + config_data = { + "github-app-id": 123456, + "github-tokens": ["token1"], + "webhook-ip": "http://localhost:5000", + "repositories": { + "test-repo": { + "name": "org/test-repo", + "labels": {"enabled-labels": ["invalid-label-category"]}, + } + }, + } + with open(config_file, "w") as f: + yaml.dump(config_data, f) + + with pytest.raises(ValueError, match="Invalid label categories in enabled-labels for repository"): + Config() + + def test_root_data_file_not_found_race_condition( + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test root_data property when file disappears after init (race condition).""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + config = Config.__new__(Config) + config.config_path = os.path.join(temp_config_dir, "config.yaml") + config.logger = Mock() + + # Remove the file to simulate race condition + os.remove(config.config_path) + + with pytest.raises(FileNotFoundError): + _ = config.root_data + + config.logger.exception.assert_called_once() + + def test_root_data_permission_error(self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch) -> None: + """Test root_data property when file cannot be read due to permissions.""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + config = Config.__new__(Config) + config.config_path = os.path.join(temp_config_dir, "config.yaml") + config.logger = Mock() + + with patch("builtins.open", side_effect=PermissionError("denied")): + with pytest.raises(PermissionError): + _ = config.root_data + + config.logger.exception.assert_called_once() + assert "Permission denied" in config.logger.exception.call_args[0][0] + + def test_root_data_generic_exception(self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch) -> None: + """Test root_data property when an unexpected exception occurs.""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + config = Config.__new__(Config) + config.config_path = os.path.join(temp_config_dir, "config.yaml") + config.logger = Mock() + + with patch("builtins.open", side_effect=OSError("disk failure")): + with pytest.raises(OSError): + _ = config.root_data + + config.logger.exception.assert_called_once() + assert "Failed to load config file" in config.logger.exception.call_args[0][0] + + def test_repository_local_data_yaml_error( + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test repository_local_data raises yaml.YAMLError for invalid YAML in repo config.""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + mock_repo = Mock() + mock_config_file = Mock() + # Invalid YAML content that will trigger yaml.YAMLError + mock_config_file.decoded_content = b"invalid: yaml: content: [" + mock_repo.get_contents.return_value = mock_config_file + + config = Config(repository="test-repo") + mock_github_api = Mock() + mock_github_api.get_repo.return_value = mock_repo + + with pytest.raises(yaml.YAMLError): + config.repository_local_data(mock_github_api, "org/test-repo") + + def test_repository_local_data_no_repository_set( + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test repository_local_data logs error when self.repository is not defined.""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + mock_logger = Mock() + config = Config(logger=mock_logger) # repository=None + mock_github_api = Mock() + + result = config.repository_local_data(mock_github_api, "org/test-repo") + + assert result == {} + mock_logger.error.assert_called_once_with( + "self.repository or self.repository_full_name is not defined" + ) + + def test_repository_local_data_no_repository_full_name_with_repo_set( + self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test repository_local_data logs error when repository_full_name is empty but self.repository is set.""" + monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) + + mock_logger = Mock() + config = Config(logger=mock_logger, repository="test-repo") + mock_github_api = Mock() + + result = config.repository_local_data(mock_github_api, "") + + assert result == {} + mock_logger.error.assert_called_once_with( + "self.repository or self.repository_full_name is not defined" + ) diff --git a/webhook_server/tests/test_pull_request_review_handler.py b/webhook_server/tests/test_pull_request_review_handler.py index a6d95e33..ab7f391a 100644 --- a/webhook_server/tests/test_pull_request_review_handler.py +++ b/webhook_server/tests/test_pull_request_review_handler.py @@ -366,3 +366,35 @@ async def test_does_not_call_test_oracle_on_non_approval( await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) mock_oracle.assert_not_called() + + +class TestPullRequestReviewHandlerWithContext: + """Test PullRequestReviewHandler with WebhookContext (ctx) set.""" + + @pytest.fixture + def mock_github_webhook_with_ctx(self) -> Mock: + mock_webhook = Mock() + mock_webhook.hook_data = { + "action": "dismissed", + "review": {"user": {"login": "reviewer"}, "state": "dismissed", "body": ""}, + } + mock_webhook.logger = Mock() + mock_webhook.log_prefix = "[TEST-CTX]" + mock_webhook.ctx = Mock() + return mock_webhook + + @pytest.fixture + def handler_with_ctx( + self, mock_github_webhook_with_ctx: Mock + ) -> PullRequestReviewHandler: + mock_owners = Mock() + return PullRequestReviewHandler(mock_github_webhook_with_ctx, mock_owners) + + @pytest.mark.asyncio + async def test_ctx_start_and_complete_step(self, handler_with_ctx: PullRequestReviewHandler) -> None: + """Test ctx.start_step and ctx.complete_step are called.""" + mock_pr = Mock(spec=PullRequest) + await handler_with_ctx.process_pull_request_review_webhook_data(mock_pr) + + handler_with_ctx.ctx.start_step.assert_called_once_with("pr_review_handler") + handler_with_ctx.ctx.complete_step.assert_called_once_with("pr_review_handler") diff --git a/webhook_server/tests/test_push_handler.py b/webhook_server/tests/test_push_handler.py index 23c0ad36..4f128a22 100644 --- a/webhook_server/tests/test_push_handler.py +++ b/webhook_server/tests/test_push_handler.py @@ -412,3 +412,172 @@ async def test_upload_to_pypi_slack_message_format(self, push_handler: PushHandl assert "published to PYPI" in call_args[1]["message"] assert call_args[1]["logger"] == push_handler.logger assert call_args[1]["log_prefix"] == push_handler.log_prefix + + +class TestPushHandlerTitleTruncation: + """Test suite for title sanitization/truncation in upload_to_pypi.""" + + @pytest.fixture + def mock_github_webhook(self) -> Mock: + mock_webhook = Mock() + mock_webhook.hook_data = {"ref": "refs/tags/v1.0.0"} + mock_webhook.logger = Mock() + mock_webhook.log_prefix = "[TEST]" + mock_webhook.repository = Mock() + mock_webhook.pypi = {"token": "test-token"} + mock_webhook.build_and_push_container = False + mock_webhook.container_release = False + mock_webhook.clone_repo_dir = "/tmp/test-repo" + mock_webhook.slack_webhook_url = "" + mock_webhook.repository_name = "test-repo" + mock_webhook.container_repository_username = "test-user" + mock_webhook.container_repository_password = "test-password" # pragma: allowlist secret + mock_webhook.token = "test-token" + mock_webhook.ctx = None + mock_webhook.custom_check_runs = [] + return mock_webhook + + @pytest.fixture + def push_handler(self, mock_github_webhook: Mock) -> PushHandler: + return PushHandler(mock_github_webhook) + + @pytest.mark.asyncio + async def test_upload_to_pypi_issue_title_truncated_when_long( + self, push_handler: PushHandler + ) -> None: + """Test that issue title is truncated to 250 chars when error message is very long.""" + with patch.object(push_handler.runner_handler, "_checkout_worktree") as mock_checkout: + with patch.object(push_handler.repository, "create_issue") as mock_create_issue: + # Create a very long error message (>250 chars) + long_error = "A" * 300 + _set_checkout_result( + mock_checkout, + (False, "/tmp/worktree-path", long_error, ""), + ) + + await push_handler.upload_to_pypi(tag_name="v1.0.0") + + mock_create_issue.assert_called_once() + call_args = mock_create_issue.call_args + title = call_args[1]["title"] + assert len(title) == 250 + assert title.endswith("...") + + +class TestPushHandlerWithContext: + """Test suite for PushHandler with WebhookContext (ctx) set.""" + + @pytest.fixture + def mock_github_webhook_with_ctx(self) -> Mock: + """Create a mock GithubWebhook instance with a ctx.""" + mock_webhook = Mock() + mock_webhook.hook_data = {"ref": "refs/tags/v2.0.0"} + mock_webhook.logger = Mock() + mock_webhook.log_prefix = "[TEST-CTX]" + mock_webhook.repository = Mock() + mock_webhook.pypi = {"token": "test-token"} + mock_webhook.build_and_push_container = True + mock_webhook.container_release = True + mock_webhook.clone_repo_dir = "/tmp/test-repo" + mock_webhook.slack_webhook_url = "" + mock_webhook.repository_name = "test-repo" + mock_webhook.container_repository_username = "test-user" + mock_webhook.container_repository_password = "test-password" # pragma: allowlist secret + mock_webhook.token = "test-token" + mock_webhook.custom_check_runs = [] + # Set ctx to a mock with start_step, fail_step, complete_step + mock_ctx = Mock() + mock_webhook.ctx = mock_ctx + return mock_webhook + + @pytest.fixture + def push_handler_with_ctx(self, mock_github_webhook_with_ctx: Mock) -> PushHandler: + return PushHandler(mock_github_webhook_with_ctx) + + @pytest.mark.asyncio + async def test_ctx_start_and_complete_step_no_tag(self, push_handler_with_ctx: PushHandler) -> None: + """Test ctx.start_step and ctx.complete_step when ref is not a tag (no-op path).""" + push_handler_with_ctx.hook_data["ref"] = "refs/heads/main" + + await push_handler_with_ctx.process_push_webhook_data() + + push_handler_with_ctx.ctx.start_step.assert_called_once_with("push_handler") + push_handler_with_ctx.ctx.complete_step.assert_called_once_with("push_handler") + + @pytest.mark.asyncio + async def test_ctx_start_and_complete_step_tag_success( + self, push_handler_with_ctx: PushHandler + ) -> None: + """Test ctx.start_step and ctx.complete_step on successful tag push.""" + with patch.object(push_handler_with_ctx, "upload_to_pypi", new_callable=AsyncMock): + with patch.object( + push_handler_with_ctx.runner_handler, "run_build_container", new_callable=AsyncMock + ): + await push_handler_with_ctx.process_push_webhook_data() + + push_handler_with_ctx.ctx.start_step.assert_called_once_with("push_handler") + push_handler_with_ctx.ctx.complete_step.assert_called_once_with("push_handler") + + @pytest.mark.asyncio + async def test_ctx_fail_step_on_pypi_exception(self, push_handler_with_ctx: PushHandler) -> None: + """Test ctx.fail_step when upload_to_pypi raises an exception.""" + pypi_error = RuntimeError("PyPI upload boom") + with patch.object( + push_handler_with_ctx, "upload_to_pypi", new_callable=AsyncMock, side_effect=pypi_error + ): + await push_handler_with_ctx.process_push_webhook_data() + + push_handler_with_ctx.ctx.start_step.assert_called_once_with("push_handler") + push_handler_with_ctx.ctx.fail_step.assert_called_once() + call_args = push_handler_with_ctx.ctx.fail_step.call_args + assert call_args[0][0] == "push_handler" + assert call_args[0][1] is pypi_error + # complete_step should NOT be called + push_handler_with_ctx.ctx.complete_step.assert_not_called() + + @pytest.mark.asyncio + async def test_ctx_fail_step_on_container_build_exception( + self, push_handler_with_ctx: PushHandler + ) -> None: + """Test ctx.fail_step when run_build_container raises an exception.""" + push_handler_with_ctx.github_webhook.pypi = {} # skip pypi path + container_error = RuntimeError("Container build boom") + with patch.object( + push_handler_with_ctx.runner_handler, + "run_build_container", + new_callable=AsyncMock, + side_effect=container_error, + ): + await push_handler_with_ctx.process_push_webhook_data() + + push_handler_with_ctx.ctx.start_step.assert_called_once_with("push_handler") + push_handler_with_ctx.ctx.fail_step.assert_called_once() + call_args = push_handler_with_ctx.ctx.fail_step.call_args + assert call_args[0][0] == "push_handler" + assert call_args[0][1] is container_error + push_handler_with_ctx.ctx.complete_step.assert_not_called() + + @pytest.mark.asyncio + async def test_pypi_exception_without_ctx(self, mock_github_webhook_with_ctx: Mock) -> None: + """Test pypi exception path returns early even when ctx is None.""" + mock_github_webhook_with_ctx.ctx = None + handler = PushHandler(mock_github_webhook_with_ctx) + with patch.object( + handler, "upload_to_pypi", new_callable=AsyncMock, side_effect=RuntimeError("boom") + ): + # Should not raise — it catches and returns + await handler.process_push_webhook_data() + + @pytest.mark.asyncio + async def test_container_exception_without_ctx(self, mock_github_webhook_with_ctx: Mock) -> None: + """Test container build exception path returns early when ctx is None.""" + mock_github_webhook_with_ctx.ctx = None + mock_github_webhook_with_ctx.pypi = {} # skip pypi + handler = PushHandler(mock_github_webhook_with_ctx) + with patch.object( + handler.runner_handler, + "run_build_container", + new_callable=AsyncMock, + side_effect=RuntimeError("container boom"), + ): + await handler.process_push_webhook_data() From 8b5f205ee78f67aff38b3bd51789fc012d62abb3 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 21:06:54 +0300 Subject: [PATCH 08/25] style: fix ruff formatting in new test files\n\nCo-authored-by: PI (claude-opus-4-6) Signed-off-by: rnetser --- webhook_server/tests/test_config.py | 16 ++++------------ .../tests/test_pull_request_review_handler.py | 4 +--- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/webhook_server/tests/test_config.py b/webhook_server/tests/test_config.py index e32e3232..7c5f3fc8 100644 --- a/webhook_server/tests/test_config.py +++ b/webhook_server/tests/test_config.py @@ -221,9 +221,7 @@ def test_repository_local_data_list_result(self, temp_config_dir: str, monkeypat assert result == {"local-setting": "value"} - def test_repository_local_data_file_not_found( - self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch - ) -> None: + def test_repository_local_data_file_not_found(self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch) -> None: """Test repository_local_data method when config file is not found.""" monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) @@ -490,9 +488,7 @@ def test_root_data_generic_exception(self, temp_config_dir: str, monkeypatch: py config.logger.exception.assert_called_once() assert "Failed to load config file" in config.logger.exception.call_args[0][0] - def test_repository_local_data_yaml_error( - self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch - ) -> None: + def test_repository_local_data_yaml_error(self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch) -> None: """Test repository_local_data raises yaml.YAMLError for invalid YAML in repo config.""" monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_config_dir) @@ -522,9 +518,7 @@ def test_repository_local_data_no_repository_set( result = config.repository_local_data(mock_github_api, "org/test-repo") assert result == {} - mock_logger.error.assert_called_once_with( - "self.repository or self.repository_full_name is not defined" - ) + mock_logger.error.assert_called_once_with("self.repository or self.repository_full_name is not defined") def test_repository_local_data_no_repository_full_name_with_repo_set( self, temp_config_dir: str, monkeypatch: pytest.MonkeyPatch @@ -539,6 +533,4 @@ def test_repository_local_data_no_repository_full_name_with_repo_set( result = config.repository_local_data(mock_github_api, "") assert result == {} - mock_logger.error.assert_called_once_with( - "self.repository or self.repository_full_name is not defined" - ) + mock_logger.error.assert_called_once_with("self.repository or self.repository_full_name is not defined") diff --git a/webhook_server/tests/test_pull_request_review_handler.py b/webhook_server/tests/test_pull_request_review_handler.py index ab7f391a..c2034ec6 100644 --- a/webhook_server/tests/test_pull_request_review_handler.py +++ b/webhook_server/tests/test_pull_request_review_handler.py @@ -384,9 +384,7 @@ def mock_github_webhook_with_ctx(self) -> Mock: return mock_webhook @pytest.fixture - def handler_with_ctx( - self, mock_github_webhook_with_ctx: Mock - ) -> PullRequestReviewHandler: + def handler_with_ctx(self, mock_github_webhook_with_ctx: Mock) -> PullRequestReviewHandler: mock_owners = Mock() return PullRequestReviewHandler(mock_github_webhook_with_ctx, mock_owners) From 1b46cd1513773718f17c966563e097ec78c54f3a Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 23 Jun 2026 21:15:34 +0300 Subject: [PATCH 09/25] style: apply ruff format to all PR files\n\nCo-authored-by: PI (claude-opus-4-6) Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 14 --- .../libs/handlers/pull_request_handler.py | 38 -------- webhook_server/tests/test_config_schema.py | 82 ---------------- .../test_prepare_retest_welcome_comment.py | 97 ------------------- .../tests/test_pull_request_handler.py | 1 - webhook_server/tests/test_push_handler.py | 24 ++--- 6 files changed, 6 insertions(+), 250 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 4bdf530a..deef20fa 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -941,20 +941,6 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: ) self.custom_check_runs: list[dict[str, Any]] = self._validate_custom_check_runs(raw_custom_checks) - raw_custom_commands = self.config.get_value( - value="custom-commands", return_on_none=[], extra_dict=repository_config - ) - if not isinstance(raw_custom_commands, list): - if raw_custom_commands is not None: - prefix = getattr(self, "log_prefix", "") - self.logger.warning( - f"{prefix} custom-commands config is not a list " - f"(got {type(raw_custom_commands).__name__}), skipping" - ) - self.custom_commands: list[dict[str, str]] = [] - else: - self.custom_commands = raw_custom_commands - _auto_users = self.config.get_value( value="auto-verified-and-merged-users", return_on_none=[], extra_dict=repository_config ) diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index f4693382..a511123a 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -2,7 +2,6 @@ import asyncio import hashlib -import re import shlex import traceback from collections.abc import Coroutine @@ -498,7 +497,6 @@ def _prepare_welcome_comment(self) -> str: {self._prepare_retest_welcome_comment} {self._prepare_container_operations_welcome_section}\ {self._prepare_cherry_pick_section}\ -{self._prepare_custom_commands_welcome_section}\ #### Label Management * `/` - Add a label to the PR @@ -844,42 +842,6 @@ def _prepare_cherry_pick_section(self) -> str: """ return "\n#### Branch Management\n* `/rebase` - Rebase this PR branch onto its base branch\n" - @property - def _prepare_custom_commands_welcome_section(self) -> str: - """Prepare the Custom Commands section for the welcome comment. - - Renders user-defined custom commands from configuration. - These are documentation-only - the server does not process them. - Invalid entries are skipped with a warning log. - """ - custom_commands: list[dict[str, str]] = self.github_webhook.custom_commands - if not custom_commands: - return "" - - lines: list[str] = ["\n#### Custom Commands"] - for cmd in custom_commands: - if not isinstance(cmd, dict): - self.logger.warning(f"{self.log_prefix} Skipping invalid custom-command entry: not a dict") - continue - - name = cmd.get("name") - description = cmd.get("description") - if not isinstance(name, str) or not isinstance(description, str) or not name or not description: - self.logger.warning(f"{self.log_prefix} Skipping custom-command with missing name or description") - continue - - if not re.fullmatch(r"[a-zA-Z0-9_-]+", name): - self.logger.warning(f"{self.log_prefix} Skipping custom-command with invalid name: {name!r}") - continue - - sanitized_description = description.replace("\n", " ").replace("\r", " ") - lines.append(f"* `/{name}` - {sanitized_description}") - - if len(lines) == 1: - return "" - - return "\n".join(lines) + "\n" - async def label_all_opened_pull_requests_merge_state_after_merged(self) -> None: """ Labels pull requests based on their mergeable state. diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index d5fd8205..6a600257 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -5,7 +5,6 @@ import pytest import yaml -from jsonschema import ValidationError, validate from webhook_server.libs.config import Config @@ -1007,84 +1006,3 @@ def test_ai_features_resolve_cherry_pick_conflicts_with_ai_repository_level( } finally: shutil.rmtree(temp_dir) - - -class TestCustomCommandsSchema: - """Tests for custom-commands schema validation.""" - - def test_schema_allows_global_custom_commands(self) -> None: - """Test that schema.yaml defines custom-commands at root level.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - assert "custom-commands" in schema["properties"] - assert schema["properties"]["custom-commands"]["type"] == "array" - - def test_schema_allows_per_repo_custom_commands(self) -> None: - """Test that schema.yaml defines custom-commands in per-repo properties.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - repo_props = schema["properties"]["repositories"]["additionalProperties"]["properties"] - assert "custom-commands" in repo_props - assert repo_props["custom-commands"]["type"] == "array" - - def test_custom_commands_valid_config(self) -> None: - """Test that valid custom-commands config passes schema validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - - config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], # pragma: allowlist secret - "custom-commands": [ - {"name": "deploy-staging", "description": "Deploy to staging"}, - {"name": "run-e2e", "description": "Run e2e tests"}, - ], - } - validate(instance=config, schema=schema) - - def test_custom_commands_rejects_missing_name(self) -> None: - """Test that custom-commands without name fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - - config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], # pragma: allowlist secret - "custom-commands": [ - {"description": "Missing name field"}, - ], - } - with pytest.raises(ValidationError): - validate(instance=config, schema=schema) - - def test_custom_commands_rejects_invalid_name_pattern(self) -> None: - """Test that custom-commands with invalid name pattern fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - - config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], # pragma: allowlist secret - "custom-commands": [ - {"name": "invalid name with spaces", "description": "Bad name"}, - ], - } - with pytest.raises(ValidationError): - validate(instance=config, schema=schema) - - def test_custom_commands_rejects_extra_properties(self) -> None: - """Test that custom-commands with extra properties fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - - config: dict[str, Any] = { - "github-tokens": ["ghp_test123"], # pragma: allowlist secret - "custom-commands": [ - {"name": "test", "description": "Test", "extra": "not allowed"}, - ], - } - with pytest.raises(ValidationError): - validate(instance=config, schema=schema) diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index 5ee2af47..c47a9da7 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -348,100 +348,3 @@ def test_retest_content_between_testing_header_and_container_section( # Should contain retest commands assert "/retest tox" in section_between, "Retest tox command should be in Testing & Validation section" - - -class TestCustomCommandsWelcomeSection: - """Tests for the custom commands welcome section feature. - - Verifies that user-defined custom commands from configuration - are rendered correctly in the welcome message. - """ - - def _create_handler( - self, - process_github_webhook: GithubWebhook, - owners_file_handler: OwnersFileHandler, - custom_commands: list[dict[str, str]] | None = None, - ) -> PullRequestHandler: - process_github_webhook.tox = False - process_github_webhook.build_and_push_container = False - process_github_webhook.pypi = False - process_github_webhook.pre_commit = False - process_github_webhook.conventional_title = False - process_github_webhook.parent_committer = "test-user" - process_github_webhook.auto_verified_and_merged_users = [] - process_github_webhook.create_issue_for_new_pr = False - process_github_webhook.issue_url_for_welcome_msg = "https://github.com/test/repo/issues/1" - process_github_webhook.minimum_lgtm = 1 - process_github_webhook.pull_request = None - - owners_file_handler.all_pull_request_approvers = ["approver1"] - owners_file_handler.all_pull_request_reviewers = ["reviewer1"] - - process_github_webhook.custom_commands = custom_commands if custom_commands is not None else [] - - return PullRequestHandler(github_webhook=process_github_webhook, owners_file_handler=owners_file_handler) - - def test_custom_commands_not_configured( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """When no custom commands are configured, welcome message should not contain the section.""" - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=[]) - welcome_msg = handler._prepare_welcome_comment() - assert "#### Custom Commands" not in welcome_msg - - def test_custom_commands_configured( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """When custom commands are configured, welcome message should contain the section and command text.""" - commands = [ - {"name": "deploy-staging", "description": "Deploy this PR to staging"}, - {"name": "run-e2e", "description": "Run e2e tests"}, - ] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) - welcome_msg = handler._prepare_welcome_comment() - - assert "#### Custom Commands" in welcome_msg - assert "/deploy-staging" in welcome_msg - assert "Deploy this PR to staging" in welcome_msg - assert "/run-e2e" in welcome_msg - assert "Run e2e tests" in welcome_msg - - def test_custom_commands_formatting( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """Each custom command should be formatted as: * `/name` - description.""" - commands = [ - {"name": "deploy-staging", "description": "Deploy this PR to staging"}, - {"name": "run-e2e", "description": "Run e2e tests"}, - ] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) - welcome_msg = handler._prepare_welcome_comment() - - assert "* `/deploy-staging` - Deploy this PR to staging" in welcome_msg - assert "* `/run-e2e` - Run e2e tests" in welcome_msg - - def test_custom_commands_header_on_own_line( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """The Custom Commands header should be preceded by a newline.""" - commands = [{"name": "deploy-staging", "description": "Deploy this PR to staging"}] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) - welcome_msg = handler._prepare_welcome_comment() - - assert "\n#### Custom Commands\n" in welcome_msg, "#### Custom Commands header should be on its own line" - - def test_custom_commands_appears_before_label_management( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """The Custom Commands section should appear before the Label Management section.""" - commands = [{"name": "deploy-staging", "description": "Deploy this PR to staging"}] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) - welcome_msg = handler._prepare_welcome_comment() - - custom_pos = welcome_msg.find("#### Custom Commands") - label_pos = welcome_msg.find("#### Label Management") - - assert custom_pos != -1, "Custom Commands section should be present" - assert label_pos != -1, "Label Management section should be present" - assert custom_pos < label_pos, "Custom Commands section should appear before Label Management section" diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index fe831f1c..4a7642a1 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -92,7 +92,6 @@ def mock_github_webhook(self) -> Mock: mock_webhook.ctx = None mock_webhook.enabled_labels = None # Default: all labels enabled mock_webhook.custom_check_runs = [] - mock_webhook.custom_commands = [] mock_webhook.ai_features = None mock_webhook.required_conversation_resolution = False mock_webhook.security_suspicious_paths = [] diff --git a/webhook_server/tests/test_push_handler.py b/webhook_server/tests/test_push_handler.py index 4f128a22..5abacb95 100644 --- a/webhook_server/tests/test_push_handler.py +++ b/webhook_server/tests/test_push_handler.py @@ -442,9 +442,7 @@ def push_handler(self, mock_github_webhook: Mock) -> PushHandler: return PushHandler(mock_github_webhook) @pytest.mark.asyncio - async def test_upload_to_pypi_issue_title_truncated_when_long( - self, push_handler: PushHandler - ) -> None: + async def test_upload_to_pypi_issue_title_truncated_when_long(self, push_handler: PushHandler) -> None: """Test that issue title is truncated to 250 chars when error message is very long.""" with patch.object(push_handler.runner_handler, "_checkout_worktree") as mock_checkout: with patch.object(push_handler.repository, "create_issue") as mock_create_issue: @@ -505,14 +503,10 @@ async def test_ctx_start_and_complete_step_no_tag(self, push_handler_with_ctx: P push_handler_with_ctx.ctx.complete_step.assert_called_once_with("push_handler") @pytest.mark.asyncio - async def test_ctx_start_and_complete_step_tag_success( - self, push_handler_with_ctx: PushHandler - ) -> None: + async def test_ctx_start_and_complete_step_tag_success(self, push_handler_with_ctx: PushHandler) -> None: """Test ctx.start_step and ctx.complete_step on successful tag push.""" with patch.object(push_handler_with_ctx, "upload_to_pypi", new_callable=AsyncMock): - with patch.object( - push_handler_with_ctx.runner_handler, "run_build_container", new_callable=AsyncMock - ): + with patch.object(push_handler_with_ctx.runner_handler, "run_build_container", new_callable=AsyncMock): await push_handler_with_ctx.process_push_webhook_data() push_handler_with_ctx.ctx.start_step.assert_called_once_with("push_handler") @@ -522,9 +516,7 @@ async def test_ctx_start_and_complete_step_tag_success( async def test_ctx_fail_step_on_pypi_exception(self, push_handler_with_ctx: PushHandler) -> None: """Test ctx.fail_step when upload_to_pypi raises an exception.""" pypi_error = RuntimeError("PyPI upload boom") - with patch.object( - push_handler_with_ctx, "upload_to_pypi", new_callable=AsyncMock, side_effect=pypi_error - ): + with patch.object(push_handler_with_ctx, "upload_to_pypi", new_callable=AsyncMock, side_effect=pypi_error): await push_handler_with_ctx.process_push_webhook_data() push_handler_with_ctx.ctx.start_step.assert_called_once_with("push_handler") @@ -536,9 +528,7 @@ async def test_ctx_fail_step_on_pypi_exception(self, push_handler_with_ctx: Push push_handler_with_ctx.ctx.complete_step.assert_not_called() @pytest.mark.asyncio - async def test_ctx_fail_step_on_container_build_exception( - self, push_handler_with_ctx: PushHandler - ) -> None: + async def test_ctx_fail_step_on_container_build_exception(self, push_handler_with_ctx: PushHandler) -> None: """Test ctx.fail_step when run_build_container raises an exception.""" push_handler_with_ctx.github_webhook.pypi = {} # skip pypi path container_error = RuntimeError("Container build boom") @@ -562,9 +552,7 @@ async def test_pypi_exception_without_ctx(self, mock_github_webhook_with_ctx: Mo """Test pypi exception path returns early even when ctx is None.""" mock_github_webhook_with_ctx.ctx = None handler = PushHandler(mock_github_webhook_with_ctx) - with patch.object( - handler, "upload_to_pypi", new_callable=AsyncMock, side_effect=RuntimeError("boom") - ): + with patch.object(handler, "upload_to_pypi", new_callable=AsyncMock, side_effect=RuntimeError("boom")): # Should not raise — it catches and returns await handler.process_push_webhook_data() From 3fe865dc39da325f5e7d58e19ecf4984d56af8e5 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 14:44:31 +0300 Subject: [PATCH 10/25] fix: address PR review comments - Extract custom-command-item to $defs, use $ref from both global and per-repo - Add traceback_str assertion to push handler context tests Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/config/schema.yaml | 46 ++++++++++------------- webhook_server/tests/test_push_handler.py | 2 + 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 70ad282c..2ee35195 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -68,6 +68,20 @@ $defs: - ai-provider - ai-model additionalProperties: false + custom-command-item: + type: object + properties: + name: + type: string + pattern: "^[a-zA-Z0-9_-]+$" + description: Command name (without the leading slash) + description: + type: string + description: Human-readable description of what the command does + required: + - name + - description + additionalProperties: false security-checks: type: object description: | @@ -261,19 +275,7 @@ properties: These are documentation-only - the server renders them in the welcome message but does NOT process them. External bots or tools handle them. items: - type: object - properties: - name: - type: string - pattern: "^[a-zA-Z0-9_-]+$" - description: Command name (without the leading slash) - description: - type: string - description: Human-readable description of what the command does - required: - - name - - description - additionalProperties: false + $ref: '#/$defs/custom-command-item' labels: type: object description: Configure which labels are enabled and their colors @@ -740,9 +742,11 @@ properties: custom-commands: type: array description: | - Custom commands to display in the PR welcome message. + Custom commands to display in the PR welcome message (per-repo override). These are documentation-only - the server renders them in the welcome message but does NOT process them. External bots or tools handle them. + An empty list is allowed to explicitly disable global custom-commands + for this repository. Examples: - name: deploy-staging @@ -750,16 +754,4 @@ properties: - name: run-e2e description: Trigger end-to-end test suite against this PR items: - type: object - properties: - name: - type: string - pattern: "^[a-zA-Z0-9_-]+$" - description: Command name (without the leading slash) - description: - type: string - description: Human-readable description of what the command does - required: - - name - - description - additionalProperties: false + $ref: '#/$defs/custom-command-item' diff --git a/webhook_server/tests/test_push_handler.py b/webhook_server/tests/test_push_handler.py index 5abacb95..150094fd 100644 --- a/webhook_server/tests/test_push_handler.py +++ b/webhook_server/tests/test_push_handler.py @@ -524,6 +524,7 @@ async def test_ctx_fail_step_on_pypi_exception(self, push_handler_with_ctx: Push call_args = push_handler_with_ctx.ctx.fail_step.call_args assert call_args[0][0] == "push_handler" assert call_args[0][1] is pypi_error + assert isinstance(call_args[0][2], str) # complete_step should NOT be called push_handler_with_ctx.ctx.complete_step.assert_not_called() @@ -545,6 +546,7 @@ async def test_ctx_fail_step_on_container_build_exception(self, push_handler_wit call_args = push_handler_with_ctx.ctx.fail_step.call_args assert call_args[0][0] == "push_handler" assert call_args[0][1] is container_error + assert isinstance(call_args[0][2], str) push_handler_with_ctx.ctx.complete_step.assert_not_called() @pytest.mark.asyncio From 3e232f384d17c6e04f024b58cc0aff0c6a6439b5 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 16:14:37 +0300 Subject: [PATCH 11/25] test: strengthen traceback assertions in push handler ctx tests Assert traceback is non-empty, contains Traceback marker and exception message. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/tests/test_push_handler.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/webhook_server/tests/test_push_handler.py b/webhook_server/tests/test_push_handler.py index 150094fd..c101792c 100644 --- a/webhook_server/tests/test_push_handler.py +++ b/webhook_server/tests/test_push_handler.py @@ -525,6 +525,9 @@ async def test_ctx_fail_step_on_pypi_exception(self, push_handler_with_ctx: Push assert call_args[0][0] == "push_handler" assert call_args[0][1] is pypi_error assert isinstance(call_args[0][2], str) + assert call_args[0][2].strip() + assert "Traceback (most recent call last)" in call_args[0][2] + assert "PyPI upload boom" in call_args[0][2] # complete_step should NOT be called push_handler_with_ctx.ctx.complete_step.assert_not_called() @@ -547,6 +550,9 @@ async def test_ctx_fail_step_on_container_build_exception(self, push_handler_wit assert call_args[0][0] == "push_handler" assert call_args[0][1] is container_error assert isinstance(call_args[0][2], str) + assert call_args[0][2].strip() + assert "Traceback (most recent call last)" in call_args[0][2] + assert "Container build boom" in call_args[0][2] push_handler_with_ctx.ctx.complete_step.assert_not_called() @pytest.mark.asyncio From a8e2782493abf48a49bedee26ef800666eeb0cbd Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 16:40:28 +0300 Subject: [PATCH 12/25] fix: restore handler code lost during rebase and add AGENTS.md docs - Restore _prepare_custom_commands_welcome_section handler and integration - Restore custom_commands config loading in github_api.py - Restore custom-commands schema tests and handler tests - Add validation tests for non-dict, missing fields, invalid name patterns - Add custom-commands section to AGENTS.md documentation Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 14 ++ .../libs/handlers/pull_request_handler.py | 38 +++++ webhook_server/tests/test_config_schema.py | 82 ++++++++++ .../test_prepare_retest_welcome_comment.py | 147 ++++++++++++++++++ .../tests/test_pull_request_handler.py | 1 + 5 files changed, 282 insertions(+) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index deef20fa..4bdf530a 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -941,6 +941,20 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: ) self.custom_check_runs: list[dict[str, Any]] = self._validate_custom_check_runs(raw_custom_checks) + raw_custom_commands = self.config.get_value( + value="custom-commands", return_on_none=[], extra_dict=repository_config + ) + if not isinstance(raw_custom_commands, list): + if raw_custom_commands is not None: + prefix = getattr(self, "log_prefix", "") + self.logger.warning( + f"{prefix} custom-commands config is not a list " + f"(got {type(raw_custom_commands).__name__}), skipping" + ) + self.custom_commands: list[dict[str, str]] = [] + else: + self.custom_commands = raw_custom_commands + _auto_users = self.config.get_value( value="auto-verified-and-merged-users", return_on_none=[], extra_dict=repository_config ) diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index a511123a..f4693382 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -2,6 +2,7 @@ import asyncio import hashlib +import re import shlex import traceback from collections.abc import Coroutine @@ -497,6 +498,7 @@ def _prepare_welcome_comment(self) -> str: {self._prepare_retest_welcome_comment} {self._prepare_container_operations_welcome_section}\ {self._prepare_cherry_pick_section}\ +{self._prepare_custom_commands_welcome_section}\ #### Label Management * `/` - Add a label to the PR @@ -842,6 +844,42 @@ def _prepare_cherry_pick_section(self) -> str: """ return "\n#### Branch Management\n* `/rebase` - Rebase this PR branch onto its base branch\n" + @property + def _prepare_custom_commands_welcome_section(self) -> str: + """Prepare the Custom Commands section for the welcome comment. + + Renders user-defined custom commands from configuration. + These are documentation-only - the server does not process them. + Invalid entries are skipped with a warning log. + """ + custom_commands: list[dict[str, str]] = self.github_webhook.custom_commands + if not custom_commands: + return "" + + lines: list[str] = ["\n#### Custom Commands"] + for cmd in custom_commands: + if not isinstance(cmd, dict): + self.logger.warning(f"{self.log_prefix} Skipping invalid custom-command entry: not a dict") + continue + + name = cmd.get("name") + description = cmd.get("description") + if not isinstance(name, str) or not isinstance(description, str) or not name or not description: + self.logger.warning(f"{self.log_prefix} Skipping custom-command with missing name or description") + continue + + if not re.fullmatch(r"[a-zA-Z0-9_-]+", name): + self.logger.warning(f"{self.log_prefix} Skipping custom-command with invalid name: {name!r}") + continue + + sanitized_description = description.replace("\n", " ").replace("\r", " ") + lines.append(f"* `/{name}` - {sanitized_description}") + + if len(lines) == 1: + return "" + + return "\n".join(lines) + "\n" + async def label_all_opened_pull_requests_merge_state_after_merged(self) -> None: """ Labels pull requests based on their mergeable state. diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index 6a600257..d5fd8205 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -5,6 +5,7 @@ import pytest import yaml +from jsonschema import ValidationError, validate from webhook_server.libs.config import Config @@ -1006,3 +1007,84 @@ def test_ai_features_resolve_cherry_pick_conflicts_with_ai_repository_level( } finally: shutil.rmtree(temp_dir) + + +class TestCustomCommandsSchema: + """Tests for custom-commands schema validation.""" + + def test_schema_allows_global_custom_commands(self) -> None: + """Test that schema.yaml defines custom-commands at root level.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + assert "custom-commands" in schema["properties"] + assert schema["properties"]["custom-commands"]["type"] == "array" + + def test_schema_allows_per_repo_custom_commands(self) -> None: + """Test that schema.yaml defines custom-commands in per-repo properties.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + repo_props = schema["properties"]["repositories"]["additionalProperties"]["properties"] + assert "custom-commands" in repo_props + assert repo_props["custom-commands"]["type"] == "array" + + def test_custom_commands_valid_config(self) -> None: + """Test that valid custom-commands config passes schema validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], # pragma: allowlist secret + "custom-commands": [ + {"name": "deploy-staging", "description": "Deploy to staging"}, + {"name": "run-e2e", "description": "Run e2e tests"}, + ], + } + validate(instance=config, schema=schema) + + def test_custom_commands_rejects_missing_name(self) -> None: + """Test that custom-commands without name fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], # pragma: allowlist secret + "custom-commands": [ + {"description": "Missing name field"}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) + + def test_custom_commands_rejects_invalid_name_pattern(self) -> None: + """Test that custom-commands with invalid name pattern fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], # pragma: allowlist secret + "custom-commands": [ + {"name": "invalid name with spaces", "description": "Bad name"}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) + + def test_custom_commands_rejects_extra_properties(self) -> None: + """Test that custom-commands with extra properties fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], # pragma: allowlist secret + "custom-commands": [ + {"name": "test", "description": "Test", "extra": "not allowed"}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index c47a9da7..0351f3ce 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -348,3 +348,150 @@ def test_retest_content_between_testing_header_and_container_section( # Should contain retest commands assert "/retest tox" in section_between, "Retest tox command should be in Testing & Validation section" + + +class TestCustomCommandsWelcomeSection: + """Tests for the custom commands welcome section feature. + + Verifies that user-defined custom commands from configuration + are rendered correctly in the welcome message. + """ + + def _create_handler( + self, + process_github_webhook: GithubWebhook, + owners_file_handler: OwnersFileHandler, + custom_commands: list[dict[str, str]] | None = None, + ) -> PullRequestHandler: + process_github_webhook.tox = False + process_github_webhook.build_and_push_container = False + process_github_webhook.pypi = False + process_github_webhook.pre_commit = False + process_github_webhook.conventional_title = False + process_github_webhook.parent_committer = "test-user" + process_github_webhook.auto_verified_and_merged_users = [] + process_github_webhook.create_issue_for_new_pr = False + process_github_webhook.issue_url_for_welcome_msg = "https://github.com/test/repo/issues/1" + process_github_webhook.minimum_lgtm = 1 + process_github_webhook.pull_request = None + + owners_file_handler.all_pull_request_approvers = ["approver1"] + owners_file_handler.all_pull_request_reviewers = ["reviewer1"] + + process_github_webhook.custom_commands = custom_commands if custom_commands is not None else [] + + return PullRequestHandler(github_webhook=process_github_webhook, owners_file_handler=owners_file_handler) + + def test_custom_commands_not_configured( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """When no custom commands are configured, welcome message should not contain the section.""" + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=[]) + welcome_msg = handler._prepare_welcome_comment() + assert "#### Custom Commands" not in welcome_msg + + def test_custom_commands_configured( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """When custom commands are configured, welcome message should contain the section and command text.""" + commands = [ + {"name": "deploy-staging", "description": "Deploy this PR to staging"}, + {"name": "run-e2e", "description": "Run e2e tests"}, + ] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + assert "#### Custom Commands" in welcome_msg + assert "/deploy-staging" in welcome_msg + assert "Deploy this PR to staging" in welcome_msg + assert "/run-e2e" in welcome_msg + assert "Run e2e tests" in welcome_msg + + def test_custom_commands_formatting( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """Each custom command should be formatted as: * `/name` - description.""" + commands = [ + {"name": "deploy-staging", "description": "Deploy this PR to staging"}, + {"name": "run-e2e", "description": "Run e2e tests"}, + ] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + assert "* `/deploy-staging` - Deploy this PR to staging" in welcome_msg + assert "* `/run-e2e` - Run e2e tests" in welcome_msg + + def test_custom_commands_header_on_own_line( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """The Custom Commands header should be preceded by a newline.""" + commands = [{"name": "deploy-staging", "description": "Deploy this PR to staging"}] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + assert "\n#### Custom Commands\n" in welcome_msg, "#### Custom Commands header should be on its own line" + + def test_custom_commands_appears_before_label_management( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """The Custom Commands section should appear before the Label Management section.""" + commands = [{"name": "deploy-staging", "description": "Deploy this PR to staging"}] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + welcome_msg = handler._prepare_welcome_comment() + + custom_pos = welcome_msg.find("#### Custom Commands") + label_pos = welcome_msg.find("#### Label Management") + + assert custom_pos != -1, "Custom Commands section should be present" + assert label_pos != -1, "Label Management section should be present" + assert custom_pos < label_pos, "Custom Commands section should appear before Label Management section" + + def test_custom_commands_skips_non_dict_entries( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """Non-dict entries in custom-commands should be skipped.""" + commands: list = [{"name": "valid-cmd", "description": "A valid command"}, "not-a-dict", 42] # type: ignore[list-item] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + section = handler._prepare_custom_commands_welcome_section + assert "/valid-cmd" in section + assert "not-a-dict" not in section + + def test_custom_commands_skips_missing_name_or_description( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """Entries missing name or description should be skipped.""" + commands = [ + {"name": "valid-cmd", "description": "A valid command"}, + {"name": "no-desc"}, + {"description": "no name"}, + {"name": "", "description": "empty name"}, + ] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) # type: ignore[arg-type] + section = handler._prepare_custom_commands_welcome_section + assert "/valid-cmd" in section + assert "no-desc" not in section + assert "no name" not in section + + def test_custom_commands_skips_invalid_name_pattern( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """Entries with names not matching [a-zA-Z0-9_-]+ should be skipped.""" + commands = [ + {"name": "valid-cmd", "description": "Valid"}, + {"name": "invalid cmd", "description": "Has space"}, + {"name": "bad/name", "description": "Has slash"}, + ] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + section = handler._prepare_custom_commands_welcome_section + assert "/valid-cmd" in section + assert "invalid cmd" not in section + assert "bad/name" not in section + + def test_custom_commands_returns_empty_when_all_invalid( + self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler + ) -> None: + """When all entries are invalid, section should be empty.""" + commands: list = ["not-a-dict", {"name": "bad name"}, {"description": "no name"}] # type: ignore[list-item] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) + section = handler._prepare_custom_commands_welcome_section + assert section == "" diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index 4a7642a1..fe831f1c 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -92,6 +92,7 @@ def mock_github_webhook(self) -> Mock: mock_webhook.ctx = None mock_webhook.enabled_labels = None # Default: all labels enabled mock_webhook.custom_check_runs = [] + mock_webhook.custom_commands = [] mock_webhook.ai_features = None mock_webhook.required_conversation_resolution = False mock_webhook.security_suspicious_paths = [] From 73d3baa73059ea87f93e3dedf8b909a1c39f7f1f Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 17:01:16 +0300 Subject: [PATCH 13/25] fix: reject empty name/description in custom-command schema Add minLength: 1 to both name and description fields in custom-command-item schema definition. This aligns schema validation with runtime behavior that skips entries with empty/falsy values. Added tests for empty name and empty description rejection. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/config/schema.yaml | 2 ++ webhook_server/tests/test_config_schema.py | 30 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 2ee35195..619fb2c9 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -74,9 +74,11 @@ $defs: name: type: string pattern: "^[a-zA-Z0-9_-]+$" + minLength: 1 description: Command name (without the leading slash) description: type: string + minLength: 1 description: Human-readable description of what the command does required: - name diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index d5fd8205..cd9f6bf5 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -1088,3 +1088,33 @@ def test_custom_commands_rejects_extra_properties(self) -> None: } with pytest.raises(ValidationError): validate(instance=config, schema=schema) + + def test_custom_commands_rejects_empty_name(self) -> None: + """Test that custom-commands with empty name fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], # pragma: allowlist secret + "custom-commands": [ + {"name": "", "description": "Test"}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) + + def test_custom_commands_rejects_empty_description(self) -> None: + """Test that custom-commands with empty description fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + + config: dict[str, Any] = { + "github-tokens": ["ghp_test123"], # pragma: allowlist secret + "custom-commands": [ + {"name": "deploy", "description": ""}, + ], + } + with pytest.raises(ValidationError): + validate(instance=config, schema=schema) From b271db23ad2b1f5196112a07d5f653f375e937a9 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 18:54:34 +0300 Subject: [PATCH 14/25] fix: use assert_awaited_once_with for async mock in ai_cli test Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/tests/test_ai_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webhook_server/tests/test_ai_cli.py b/webhook_server/tests/test_ai_cli.py index bdcef3b7..b9c1612b 100644 --- a/webhook_server/tests/test_ai_cli.py +++ b/webhook_server/tests/test_ai_cli.py @@ -74,7 +74,7 @@ async def test_call_ai_sidecar_available(self) -> None: ) assert result.success is True assert result.text == "hello" - mock_call.assert_called_once_with( + mock_call.assert_awaited_once_with( prompt="test", ai_provider="claude", ai_model="sonnet", From 542bac6da445a385003e417e7a5e9dae5bddec13 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 19:17:02 +0300 Subject: [PATCH 15/25] refactor: move custom-commands validation to load time and add markdown escaping Address human review comments: - Move runtime validation from handler to GithubWebhook._validate_custom_commands() matching _validate_custom_check_runs() pattern (myakove review #2) - Add markdown character escaping for description field to prevent injection via [ ] ( ) ! ` characters (myakove review #3) - Remove unused 're' import from pull_request_handler.py - Replace handler validation tests with load-time validator tests - Add test for markdown escaping in descriptions Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 64 +++++++-- .../libs/handlers/pull_request_handler.py | 38 +++-- .../test_prepare_retest_welcome_comment.py | 131 ++++++++++++------ 3 files changed, 161 insertions(+), 72 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 4bdf530a..42f61ad5 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -944,16 +944,7 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: raw_custom_commands = self.config.get_value( value="custom-commands", return_on_none=[], extra_dict=repository_config ) - if not isinstance(raw_custom_commands, list): - if raw_custom_commands is not None: - prefix = getattr(self, "log_prefix", "") - self.logger.warning( - f"{prefix} custom-commands config is not a list " - f"(got {type(raw_custom_commands).__name__}), skipping" - ) - self.custom_commands: list[dict[str, str]] = [] - else: - self.custom_commands = raw_custom_commands + self.custom_commands: list[dict[str, str]] = self._validate_custom_commands(raw_custom_commands) _auto_users = self.config.get_value( value="auto-verified-and-merged-users", return_on_none=[], extra_dict=repository_config @@ -1610,6 +1601,59 @@ def _validate_custom_check_runs(self, raw_checks: object) -> list[dict[str, Any] return validated_checks + def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str]]: + """Validate custom commands configuration at load time. + + Validates each custom command entry and returns only valid ones: + - Checks that entry is a dict with 'name' and 'description' string fields + - Verifies name matches the safe pattern [a-zA-Z0-9_-]+ + - Logs warnings for invalid entries and skips them + + Args: + raw_commands: Custom command configurations from config (should be a list) + + Returns: + List of validated custom command configurations + """ + if not isinstance(raw_commands, list): + if raw_commands is not None: + prefix = getattr(self, "log_prefix", "") + self.logger.warning( + f"{prefix} custom-commands config is not a list (got {type(raw_commands).__name__}), skipping" + ) + return [] + + safe_name_pattern = re.compile(r"^[a-zA-Z0-9_-]+$") + validated: list[dict[str, str]] = [] + + for cmd in raw_commands: + if not isinstance(cmd, dict): + self.logger.warning("Custom command entry is not a mapping, skipping") + continue + + name = cmd.get("name") + description = cmd.get("description") + if not isinstance(name, str) or not name: + self.logger.warning("Custom command missing or invalid 'name', skipping") + continue + + if not isinstance(description, str) or not description: + self.logger.warning(f"Custom command '{name}' missing or invalid 'description', skipping") + continue + + if not safe_name_pattern.match(name): + self.logger.warning(f"Custom command name {name!r} does not match safe pattern, skipping") + continue + + validated.append({"name": name, "description": description}) + + if validated: + self.logger.info(f"Loaded {len(validated)} custom command(s): {[c['name'] for c in validated]}") + if validated and len(validated) < len(raw_commands): + self.logger.warning(f"Skipped {len(raw_commands) - len(validated)} invalid custom command(s)") + + return validated + def __del__(self) -> None: """Remove the shared clone directory when the webhook object is destroyed. diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index f4693382..08d00ecc 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -2,7 +2,6 @@ import asyncio import hashlib -import re import shlex import traceback from collections.abc import Coroutine @@ -844,13 +843,25 @@ def _prepare_cherry_pick_section(self) -> str: """ return "\n#### Branch Management\n* `/rebase` - Rebase this PR branch onto its base branch\n" + @staticmethod + def _escape_markdown(text: str) -> str: + """Escape markdown special characters in text. + + Prevents markdown injection when inserting user-provided text + into PR comments. Escapes characters that could create links, + images, or inline code. + """ + for char in ("[", "]", "(", ")", "!", "`"): + text = text.replace(char, f"\\{char}") + return text + @property def _prepare_custom_commands_welcome_section(self) -> str: """Prepare the Custom Commands section for the welcome comment. Renders user-defined custom commands from configuration. These are documentation-only - the server does not process them. - Invalid entries are skipped with a warning log. + Commands are validated at load time by GithubWebhook._validate_custom_commands(). """ custom_commands: list[dict[str, str]] = self.github_webhook.custom_commands if not custom_commands: @@ -858,25 +869,10 @@ def _prepare_custom_commands_welcome_section(self) -> str: lines: list[str] = ["\n#### Custom Commands"] for cmd in custom_commands: - if not isinstance(cmd, dict): - self.logger.warning(f"{self.log_prefix} Skipping invalid custom-command entry: not a dict") - continue - - name = cmd.get("name") - description = cmd.get("description") - if not isinstance(name, str) or not isinstance(description, str) or not name or not description: - self.logger.warning(f"{self.log_prefix} Skipping custom-command with missing name or description") - continue - - if not re.fullmatch(r"[a-zA-Z0-9_-]+", name): - self.logger.warning(f"{self.log_prefix} Skipping custom-command with invalid name: {name!r}") - continue - - sanitized_description = description.replace("\n", " ").replace("\r", " ") - lines.append(f"* `/{name}` - {sanitized_description}") - - if len(lines) == 1: - return "" + name = cmd["name"] + description = cmd["description"] + sanitized = self._escape_markdown(description.replace("\n", " ").replace("\r", " ")) + lines.append(f"* `/{name}` - {sanitized}") return "\n".join(lines) + "\n" diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index 0351f3ce..ffbe9982 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -2,15 +2,16 @@ import re from typing import TYPE_CHECKING, TypedDict, Unpack +from unittest.mock import Mock import pytest +from webhook_server.libs.github_api import GithubWebhook from webhook_server.libs.handlers.pull_request_handler import PullRequestHandler if TYPE_CHECKING: from github.PullRequest import PullRequest - from webhook_server.libs.github_api import GithubWebhook from webhook_server.libs.handlers.owners_files_handler import OwnersFileHandler @@ -446,52 +447,100 @@ def test_custom_commands_appears_before_label_management( assert label_pos != -1, "Label Management section should be present" assert custom_pos < label_pos, "Custom Commands section should appear before Label Management section" - def test_custom_commands_skips_non_dict_entries( + def test_custom_commands_escapes_markdown_in_description( self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler ) -> None: - """Non-dict entries in custom-commands should be skipped.""" - commands: list = [{"name": "valid-cmd", "description": "A valid command"}, "not-a-dict", 42] # type: ignore[list-item] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) - section = handler._prepare_custom_commands_welcome_section - assert "/valid-cmd" in section - assert "not-a-dict" not in section - - def test_custom_commands_skips_missing_name_or_description( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """Entries missing name or description should be skipped.""" + """Markdown special characters in description should be escaped.""" commands = [ - {"name": "valid-cmd", "description": "A valid command"}, - {"name": "no-desc"}, - {"description": "no name"}, - {"name": "", "description": "empty name"}, + {"name": "deploy", "description": "Deploy [this](link) and `code` with !image"}, ] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) # type: ignore[arg-type] + handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) section = handler._prepare_custom_commands_welcome_section - assert "/valid-cmd" in section - assert "no-desc" not in section - assert "no name" not in section - - def test_custom_commands_skips_invalid_name_pattern( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """Entries with names not matching [a-zA-Z0-9_-]+ should be skipped.""" - commands = [ + assert r"\[this\]\(link\)" in section + assert r"\`code\`" in section + assert r"\!image" in section + + +class TestValidateCustomCommands: + """Test suite for GithubWebhook._validate_custom_commands() load-time validation.""" + + @pytest.fixture + def mock_webhook(self) -> Mock: + """Create a mock GithubWebhook instance for validation testing.""" + mock = Mock() + mock.logger = Mock() + mock.log_prefix = "[TEST]" + return mock + + def test_valid_commands(self, mock_webhook: Mock) -> None: + """Valid commands should pass validation.""" + raw = [ + {"name": "deploy-staging", "description": "Deploy to staging"}, + {"name": "run_e2e", "description": "Run end-to-end tests"}, + ] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert len(result) == 2 + assert result[0]["name"] == "deploy-staging" + assert result[1]["name"] == "run_e2e" + + def test_not_a_list(self, mock_webhook: Mock) -> None: + """Non-list input should return empty list and log warning.""" + result = GithubWebhook._validate_custom_commands(mock_webhook, "not-a-list") + assert result == [] + mock_webhook.logger.warning.assert_called_once() + + def test_none_input(self, mock_webhook: Mock) -> None: + """None input should return empty list without logging.""" + result = GithubWebhook._validate_custom_commands(mock_webhook, None) + assert result == [] + mock_webhook.logger.warning.assert_not_called() + + def test_empty_list(self, mock_webhook: Mock) -> None: + """Empty list should return empty list.""" + result = GithubWebhook._validate_custom_commands(mock_webhook, []) + assert result == [] + + def test_skips_non_dict_entries(self, mock_webhook: Mock) -> None: + """Non-dict entries should be skipped with warning.""" + raw = [{"name": "valid", "description": "Valid"}, "not-a-dict", 42] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert len(result) == 1 + assert result[0]["name"] == "valid" + + def test_skips_missing_name(self, mock_webhook: Mock) -> None: + """Entries without name should be skipped.""" + raw = [{"description": "no name"}, {"name": "valid", "description": "Valid"}] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert len(result) == 1 + assert result[0]["name"] == "valid" + + def test_skips_missing_description(self, mock_webhook: Mock) -> None: + """Entries without description should be skipped.""" + raw = [{"name": "no-desc"}, {"name": "valid", "description": "Valid"}] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert len(result) == 1 + assert result[0]["name"] == "valid" + + def test_skips_empty_name(self, mock_webhook: Mock) -> None: + """Entries with empty name should be skipped.""" + raw = [{"name": "", "description": "empty name"}] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert result == [] + + def test_skips_invalid_name_pattern(self, mock_webhook: Mock) -> None: + """Names not matching [a-zA-Z0-9_-]+ should be skipped.""" + raw = [ {"name": "valid-cmd", "description": "Valid"}, {"name": "invalid cmd", "description": "Has space"}, {"name": "bad/name", "description": "Has slash"}, + {"name": "bad", "description": "Has angle brackets"}, ] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) - section = handler._prepare_custom_commands_welcome_section - assert "/valid-cmd" in section - assert "invalid cmd" not in section - assert "bad/name" not in section - - def test_custom_commands_returns_empty_when_all_invalid( - self, process_github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler - ) -> None: - """When all entries are invalid, section should be empty.""" - commands: list = ["not-a-dict", {"name": "bad name"}, {"description": "no name"}] # type: ignore[list-item] - handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) - section = handler._prepare_custom_commands_welcome_section - assert section == "" + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert len(result) == 1 + assert result[0]["name"] == "valid-cmd" + + def test_all_invalid_returns_empty(self, mock_webhook: Mock) -> None: + """When all entries are invalid, should return empty list.""" + raw = ["not-a-dict", {"name": "bad name"}, {"description": "no name"}] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert result == [] From 929052471c97464aae7cce2f074c77aa9ea109a4 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 19:25:59 +0300 Subject: [PATCH 16/25] fix: add log_prefix to all custom-commands validation warnings Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 42f61ad5..e2296f24 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -1623,34 +1623,35 @@ def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str] ) return [] + prefix = getattr(self, "log_prefix", "") safe_name_pattern = re.compile(r"^[a-zA-Z0-9_-]+$") validated: list[dict[str, str]] = [] for cmd in raw_commands: if not isinstance(cmd, dict): - self.logger.warning("Custom command entry is not a mapping, skipping") + self.logger.warning(f"{prefix} Custom command entry is not a mapping, skipping") continue name = cmd.get("name") description = cmd.get("description") if not isinstance(name, str) or not name: - self.logger.warning("Custom command missing or invalid 'name', skipping") + self.logger.warning(f"{prefix} Custom command missing or invalid 'name', skipping") continue if not isinstance(description, str) or not description: - self.logger.warning(f"Custom command '{name}' missing or invalid 'description', skipping") + self.logger.warning(f"{prefix} Custom command '{name}' missing or invalid 'description', skipping") continue if not safe_name_pattern.match(name): - self.logger.warning(f"Custom command name {name!r} does not match safe pattern, skipping") + self.logger.warning(f"{prefix} Custom command name {name!r} does not match safe pattern, skipping") continue validated.append({"name": name, "description": description}) if validated: - self.logger.info(f"Loaded {len(validated)} custom command(s): {[c['name'] for c in validated]}") + self.logger.info(f"{prefix} Loaded {len(validated)} custom command(s): {[c['name'] for c in validated]}") if validated and len(validated) < len(raw_commands): - self.logger.warning(f"Skipped {len(raw_commands) - len(validated)} invalid custom command(s)") + self.logger.warning(f"{prefix} Skipped {len(raw_commands) - len(validated)} invalid custom command(s)") return validated From 97abc9acf664059952e1c576abd6138eb750fcf9 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 20:04:23 +0300 Subject: [PATCH 17/25] docs: add custom-commands section to AGENTS.md Document the custom-commands configuration option including schema, validation, handler, config loading, and per-repo override behavior. Addresses myakove review comments about missing AGENTS.md documentation. Signed-off-by: rnetser Assisted-by: Claude --- CLAUDE.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index b16979a0..651f8b63 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -695,6 +695,20 @@ AI-powered enhancements controlled by `ai-features` config (global or per-repo). - `VERTEX_CLAUDE_1M=true` — enables Claude 1M context window models via Vertex AI - `GOOGLE_APPLICATION_CREDENTIALS` — already set for Vertex AI access +### Custom Commands + +User-defined commands rendered in the PR welcome message. Documentation-only — the server displays them but does NOT process them. External bots/tools handle them independently. + +**Schema:** `webhook_server/config/schema.yaml` (`custom-commands` at global level, `$defs.custom-command-item` for DRY) + +**Config:** Global (`config.yaml`) or per-repo (`.github-webhook-server.yaml`). Per-repo merges with global; an empty list (`custom-commands: []`) disables global defaults. + +**Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`). Invalid entries are logged and skipped. + +**Handler:** `PullRequestHandler._prepare_custom_commands_welcome_section` in `webhook_server/libs/handlers/pull_request_handler.py` — renders a "Custom Commands" section with each command as `- **\/command-name**: description`. Descriptions are markdown-escaped via `_escape_markdown()`. + +**Config loading:** `self.custom_commands` loaded in `GithubWebhook._repo_data_from_config()` via `self.config.get_value("custom-commands", ...)` with `extra_dict=repository_config` for per-repo override support. + ### Sidecar Architecture **`sidecar-helper/`** — Node.js pi-sidecar bridge that provides AI provider integration (cherry-pick conflict resolution, conventional title suggestions). Contains a minimal TypeScript wrapper that imports and starts the `@myk-org/pi-sidecar` server. From 7f2fdb47cae2c2a619735110ae89d431d637f843 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 20:13:18 +0300 Subject: [PATCH 18/25] docs: fix custom-commands AGENTS.md inaccuracies - Fix "merges with global" -> "overrides global (no list merge)" Config.get_value() returns repo-local value when present, no merge. - Fix rendering format: actual is `* \`/{name}\` - description`, not `- **/command-name**: description`. Signed-off-by: rnetser Assisted-by: Claude --- CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 651f8b63..e8e07257 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -701,11 +701,11 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Schema:** `webhook_server/config/schema.yaml` (`custom-commands` at global level, `$defs.custom-command-item` for DRY) -**Config:** Global (`config.yaml`) or per-repo (`.github-webhook-server.yaml`). Per-repo merges with global; an empty list (`custom-commands: []`) disables global defaults. +**Config:** Global (`config.yaml`) or per-repo (`.github-webhook-server.yaml`). Per-repo overrides global (no list merge); an empty list (`custom-commands: []`) disables global defaults. **Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`). Invalid entries are logged and skipped. -**Handler:** `PullRequestHandler._prepare_custom_commands_welcome_section` in `webhook_server/libs/handlers/pull_request_handler.py` — renders a "Custom Commands" section with each command as `- **\/command-name**: description`. Descriptions are markdown-escaped via `_escape_markdown()`. +**Handler:** `PullRequestHandler._prepare_custom_commands_welcome_section` in `webhook_server/libs/handlers/pull_request_handler.py` — renders a "Custom Commands" section with each command as `` * `/{name}` - description ``. Descriptions are markdown-escaped via `_escape_markdown()`. **Config loading:** `self.custom_commands` loaded in `GithubWebhook._repo_data_from_config()` via `self.config.get_value("custom-commands", ...)` with `extra_dict=repository_config` for per-repo override support. From 18a055d0ba9ec791a1d8e2b6c6bd8dd28143aead Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 20:20:00 +0300 Subject: [PATCH 19/25] docs: document all 3 config resolution layers for custom-commands Document the full resolution order: (1) repo-local .github-webhook-server.yaml, (2) repositories..custom-commands in config.yaml, (3) root-level custom-commands in config.yaml. First match wins, no list merge between layers. Signed-off-by: rnetser Assisted-by: Claude --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index e8e07257..1c4d6de6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -701,7 +701,7 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Schema:** `webhook_server/config/schema.yaml` (`custom-commands` at global level, `$defs.custom-command-item` for DRY) -**Config:** Global (`config.yaml`) or per-repo (`.github-webhook-server.yaml`). Per-repo overrides global (no list merge); an empty list (`custom-commands: []`) disables global defaults. +**Config:** Three resolution layers (first match wins, no list merge): (1) repo-local `.github-webhook-server.yaml`, (2) `repositories..custom-commands` in `config.yaml`, (3) root-level `custom-commands` in `config.yaml`. An empty list (`custom-commands: []`) at any layer disables lower layers. **Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`). Invalid entries are logged and skipped. From 1e0486ac0573cde031f2d07bf42cfe6ff00ffcd7 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 24 Jun 2026 20:26:35 +0300 Subject: [PATCH 20/25] docs: clarify empty list only valid at per-repo layers Root-level schema requires minItems: 1, so custom-commands: [] is only valid at per-repo layers (.github-webhook-server.yaml or repositories. in config.yaml). Signed-off-by: rnetser Assisted-by: Claude --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1c4d6de6..214b219c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -701,7 +701,7 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Schema:** `webhook_server/config/schema.yaml` (`custom-commands` at global level, `$defs.custom-command-item` for DRY) -**Config:** Three resolution layers (first match wins, no list merge): (1) repo-local `.github-webhook-server.yaml`, (2) `repositories..custom-commands` in `config.yaml`, (3) root-level `custom-commands` in `config.yaml`. An empty list (`custom-commands: []`) at any layer disables lower layers. +**Config:** Three resolution layers (first match wins, no list merge): (1) repo-local `.github-webhook-server.yaml`, (2) `repositories..custom-commands` in `config.yaml`, (3) root-level `custom-commands` in `config.yaml`. Per-repo layers can use an empty list (`custom-commands: []`) to disable global defaults; the root-level schema requires `minItems: 1`. **Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`). Invalid entries are logged and skipped. From 003eeb12715a4f110a0f001959f07bafa2bfe4d6 Mon Sep 17 00:00:00 2001 From: rnetser Date: Thu, 25 Jun 2026 17:16:29 +0300 Subject: [PATCH 21/25] fix: address review comments on custom-commands validation - Extract schema loading into pytest fixture in TestCustomCommandsSchema (DRY) - Add duplicate command name detection in _validate_custom_commands() - Fix suppressed warning when all custom commands are invalid - Add test for duplicate command name detection - Update AGENTS.md validation description Signed-off-by: rnetser Assisted-by: Claude --- CLAUDE.md | 2 +- uv.lock | 2 + webhook_server/libs/github_api.py | 8 ++- webhook_server/tests/test_config_schema.py | 50 +++++-------------- .../test_prepare_retest_welcome_comment.py | 16 ++++++ 5 files changed, 39 insertions(+), 39 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 214b219c..e5936a0f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -703,7 +703,7 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Config:** Three resolution layers (first match wins, no list merge): (1) repo-local `.github-webhook-server.yaml`, (2) `repositories..custom-commands` in `config.yaml`, (3) root-level `custom-commands` in `config.yaml`. Per-repo layers can use an empty list (`custom-commands: []`) to disable global defaults; the root-level schema requires `minItems: 1`. -**Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`). Invalid entries are logged and skipped. +**Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`; duplicate names are rejected, keeping only the first occurrence). Invalid and duplicate entries are logged and skipped. **Handler:** `PullRequestHandler._prepare_custom_commands_welcome_section` in `webhook_server/libs/handlers/pull_request_handler.py` — renders a "Custom Commands" section with each command as `` * `/{name}` - description ``. Descriptions are markdown-escaped via `_escape_markdown()`. diff --git a/uv.lock b/uv.lock index c9d5bfec..5ab2d64b 100644 --- a/uv.lock +++ b/uv.lock @@ -515,6 +515,7 @@ dev = [ { name = "types-requests" }, ] tests = [ + { name = "jsonschema" }, { name = "psutil" }, { name = "pytest-asyncio" }, { name = "pytest-xdist" }, @@ -564,6 +565,7 @@ dev = [ { name = "types-requests", specifier = ">=2.32.4.20250611" }, ] tests = [ + { name = "jsonschema", specifier = ">=4.0.0" }, { name = "psutil", specifier = ">=7.0.0" }, { name = "pytest-asyncio", specifier = ">=0.26.0" }, { name = "pytest-xdist", specifier = ">=3.7.0" }, diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index e2296f24..2dc210c9 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -1625,6 +1625,7 @@ def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str] prefix = getattr(self, "log_prefix", "") safe_name_pattern = re.compile(r"^[a-zA-Z0-9_-]+$") + seen_names: set[str] = set() validated: list[dict[str, str]] = [] for cmd in raw_commands: @@ -1646,11 +1647,16 @@ def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str] self.logger.warning(f"{prefix} Custom command name {name!r} does not match safe pattern, skipping") continue + if name in seen_names: + self.logger.warning(f"{prefix} Custom command name {name!r} is duplicated, skipping") + continue + seen_names.add(name) + validated.append({"name": name, "description": description}) if validated: self.logger.info(f"{prefix} Loaded {len(validated)} custom command(s): {[c['name'] for c in validated]}") - if validated and len(validated) < len(raw_commands): + if len(validated) < len(raw_commands): self.logger.warning(f"{prefix} Skipped {len(raw_commands) - len(validated)} invalid custom command(s)") return validated diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index cd9f6bf5..6080f760 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -1012,29 +1012,25 @@ def test_ai_features_resolve_cherry_pick_conflicts_with_ai_repository_level( class TestCustomCommandsSchema: """Tests for custom-commands schema validation.""" - def test_schema_allows_global_custom_commands(self) -> None: - """Test that schema.yaml defines custom-commands at root level.""" + @pytest.fixture + def schema(self) -> dict[str, Any]: schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") with open(schema_path) as f: - schema = yaml.safe_load(f) + return yaml.safe_load(f) + + def test_schema_allows_global_custom_commands(self, schema: dict[str, Any]) -> None: + """Test that schema.yaml defines custom-commands at root level.""" assert "custom-commands" in schema["properties"] assert schema["properties"]["custom-commands"]["type"] == "array" - def test_schema_allows_per_repo_custom_commands(self) -> None: + def test_schema_allows_per_repo_custom_commands(self, schema: dict[str, Any]) -> None: """Test that schema.yaml defines custom-commands in per-repo properties.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) repo_props = schema["properties"]["repositories"]["additionalProperties"]["properties"] assert "custom-commands" in repo_props assert repo_props["custom-commands"]["type"] == "array" - def test_custom_commands_valid_config(self) -> None: + def test_custom_commands_valid_config(self, schema: dict[str, Any]) -> None: """Test that valid custom-commands config passes schema validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1044,12 +1040,8 @@ def test_custom_commands_valid_config(self) -> None: } validate(instance=config, schema=schema) - def test_custom_commands_rejects_missing_name(self) -> None: + def test_custom_commands_rejects_missing_name(self, schema: dict[str, Any]) -> None: """Test that custom-commands without name fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1059,12 +1051,8 @@ def test_custom_commands_rejects_missing_name(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_invalid_name_pattern(self) -> None: + def test_custom_commands_rejects_invalid_name_pattern(self, schema: dict[str, Any]) -> None: """Test that custom-commands with invalid name pattern fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1074,12 +1062,8 @@ def test_custom_commands_rejects_invalid_name_pattern(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_extra_properties(self) -> None: + def test_custom_commands_rejects_extra_properties(self, schema: dict[str, Any]) -> None: """Test that custom-commands with extra properties fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1089,12 +1073,8 @@ def test_custom_commands_rejects_extra_properties(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_empty_name(self) -> None: + def test_custom_commands_rejects_empty_name(self, schema: dict[str, Any]) -> None: """Test that custom-commands with empty name fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1104,12 +1084,8 @@ def test_custom_commands_rejects_empty_name(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_empty_description(self) -> None: + def test_custom_commands_rejects_empty_description(self, schema: dict[str, Any]) -> None: """Test that custom-commands with empty description fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index ffbe9982..00548888 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -539,6 +539,22 @@ def test_skips_invalid_name_pattern(self, mock_webhook: Mock) -> None: assert len(result) == 1 assert result[0]["name"] == "valid-cmd" + def test_skips_duplicate_names(self, mock_webhook: Mock) -> None: + """Duplicate command names should be skipped with warning.""" + raw = [ + {"name": "deploy", "description": "First"}, + {"name": "deploy", "description": "Second"}, + {"name": "unique", "description": "Third"}, + ] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert len(result) == 2 + assert result[0]["name"] == "deploy" + assert result[0]["description"] == "First" + assert result[1]["name"] == "unique" + mock_webhook.logger.warning.assert_any_call( + "[TEST] Custom command name 'deploy' is duplicated, skipping" + ) + def test_all_invalid_returns_empty(self, mock_webhook: Mock) -> None: """When all entries are invalid, should return empty list.""" raw = ["not-a-dict", {"name": "bad name"}, {"description": "no name"}] From 36c6adcc6ba8934dc96e4575602065b448357c81 Mon Sep 17 00:00:00 2001 From: rnetser Date: Thu, 25 Jun 2026 17:45:23 +0300 Subject: [PATCH 22/25] style: format test_prepare_retest_welcome_comment.py Co-authored-by: PI (claude-opus-4-6) Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/tests/test_prepare_retest_welcome_comment.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index 00548888..3fff6d87 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -551,9 +551,7 @@ def test_skips_duplicate_names(self, mock_webhook: Mock) -> None: assert result[0]["name"] == "deploy" assert result[0]["description"] == "First" assert result[1]["name"] == "unique" - mock_webhook.logger.warning.assert_any_call( - "[TEST] Custom command name 'deploy' is duplicated, skipping" - ) + mock_webhook.logger.warning.assert_any_call("[TEST] Custom command name 'deploy' is duplicated, skipping") def test_all_invalid_returns_empty(self, mock_webhook: Mock) -> None: """When all entries are invalid, should return empty list.""" From 6b8ee6ea017c574eab50a737949e1621fd5f8474 Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 28 Jun 2026 10:39:30 +0300 Subject: [PATCH 23/25] docs: update sidecar Docker base image version to node:26-slim Signed-off-by: rnetser Co-authored-by: Claude --- CLAUDE.md | 4 +- uv.lock | 2 - webhook_server/libs/github_api.py | 8 +-- webhook_server/tests/test_config_schema.py | 50 ++++++++++++++----- .../test_prepare_retest_welcome_comment.py | 14 ------ 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e5936a0f..a3dcb20a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -703,7 +703,7 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Config:** Three resolution layers (first match wins, no list merge): (1) repo-local `.github-webhook-server.yaml`, (2) `repositories..custom-commands` in `config.yaml`, (3) root-level `custom-commands` in `config.yaml`. Per-repo layers can use an empty list (`custom-commands: []`) to disable global defaults; the root-level schema requires `minItems: 1`. -**Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`; duplicate names are rejected, keeping only the first occurrence). Invalid and duplicate entries are logged and skipped. +**Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`). Invalid entries are logged and skipped. **Handler:** `PullRequestHandler._prepare_custom_commands_welcome_section` in `webhook_server/libs/handlers/pull_request_handler.py` — renders a "Custom Commands" section with each command as `` * `/{name}` - description ``. Descriptions are markdown-escaped via `_escape_markdown()`. @@ -726,7 +726,7 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Dual healthcheck:** Dockerfile `HEALTHCHECK` verifies both the webhook server (`:5000/webhook_server/healthcheck`) and the sidecar (`:${SIDECAR_PORT}/health`). Container is unhealthy if either fails. -**Docker build:** Multi-stage — `sidecar-builder` stage (node:22-slim) runs `npm ci`, `npx tsc`, `npm prune --omit=dev`. Final stage copies only `dist/`, `node_modules/` (production), and `package.json`. +**Docker build:** Multi-stage — `sidecar-builder` stage (node:26-slim) runs `npm ci`, `npx tsc`, `npm prune --omit=dev`. Final stage copies only `dist/`, `node_modules/` (production), and `package.json`. **Security note:** The AI conflict resolution prompt includes commit messages from the PR (user-controlled text). Prompt injection risk is mitigated by restricting the AI to read-only diff --git a/uv.lock b/uv.lock index 5ab2d64b..c9d5bfec 100644 --- a/uv.lock +++ b/uv.lock @@ -515,7 +515,6 @@ dev = [ { name = "types-requests" }, ] tests = [ - { name = "jsonschema" }, { name = "psutil" }, { name = "pytest-asyncio" }, { name = "pytest-xdist" }, @@ -565,7 +564,6 @@ dev = [ { name = "types-requests", specifier = ">=2.32.4.20250611" }, ] tests = [ - { name = "jsonschema", specifier = ">=4.0.0" }, { name = "psutil", specifier = ">=7.0.0" }, { name = "pytest-asyncio", specifier = ">=0.26.0" }, { name = "pytest-xdist", specifier = ">=3.7.0" }, diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 2dc210c9..e2296f24 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -1625,7 +1625,6 @@ def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str] prefix = getattr(self, "log_prefix", "") safe_name_pattern = re.compile(r"^[a-zA-Z0-9_-]+$") - seen_names: set[str] = set() validated: list[dict[str, str]] = [] for cmd in raw_commands: @@ -1647,16 +1646,11 @@ def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str] self.logger.warning(f"{prefix} Custom command name {name!r} does not match safe pattern, skipping") continue - if name in seen_names: - self.logger.warning(f"{prefix} Custom command name {name!r} is duplicated, skipping") - continue - seen_names.add(name) - validated.append({"name": name, "description": description}) if validated: self.logger.info(f"{prefix} Loaded {len(validated)} custom command(s): {[c['name'] for c in validated]}") - if len(validated) < len(raw_commands): + if validated and len(validated) < len(raw_commands): self.logger.warning(f"{prefix} Skipped {len(raw_commands) - len(validated)} invalid custom command(s)") return validated diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index 6080f760..cd9f6bf5 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -1012,25 +1012,29 @@ def test_ai_features_resolve_cherry_pick_conflicts_with_ai_repository_level( class TestCustomCommandsSchema: """Tests for custom-commands schema validation.""" - @pytest.fixture - def schema(self) -> dict[str, Any]: + def test_schema_allows_global_custom_commands(self) -> None: + """Test that schema.yaml defines custom-commands at root level.""" schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") with open(schema_path) as f: - return yaml.safe_load(f) - - def test_schema_allows_global_custom_commands(self, schema: dict[str, Any]) -> None: - """Test that schema.yaml defines custom-commands at root level.""" + schema = yaml.safe_load(f) assert "custom-commands" in schema["properties"] assert schema["properties"]["custom-commands"]["type"] == "array" - def test_schema_allows_per_repo_custom_commands(self, schema: dict[str, Any]) -> None: + def test_schema_allows_per_repo_custom_commands(self) -> None: """Test that schema.yaml defines custom-commands in per-repo properties.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) repo_props = schema["properties"]["repositories"]["additionalProperties"]["properties"] assert "custom-commands" in repo_props assert repo_props["custom-commands"]["type"] == "array" - def test_custom_commands_valid_config(self, schema: dict[str, Any]) -> None: + def test_custom_commands_valid_config(self) -> None: """Test that valid custom-commands config passes schema validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1040,8 +1044,12 @@ def test_custom_commands_valid_config(self, schema: dict[str, Any]) -> None: } validate(instance=config, schema=schema) - def test_custom_commands_rejects_missing_name(self, schema: dict[str, Any]) -> None: + def test_custom_commands_rejects_missing_name(self) -> None: """Test that custom-commands without name fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1051,8 +1059,12 @@ def test_custom_commands_rejects_missing_name(self, schema: dict[str, Any]) -> N with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_invalid_name_pattern(self, schema: dict[str, Any]) -> None: + def test_custom_commands_rejects_invalid_name_pattern(self) -> None: """Test that custom-commands with invalid name pattern fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1062,8 +1074,12 @@ def test_custom_commands_rejects_invalid_name_pattern(self, schema: dict[str, An with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_extra_properties(self, schema: dict[str, Any]) -> None: + def test_custom_commands_rejects_extra_properties(self) -> None: """Test that custom-commands with extra properties fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1073,8 +1089,12 @@ def test_custom_commands_rejects_extra_properties(self, schema: dict[str, Any]) with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_empty_name(self, schema: dict[str, Any]) -> None: + def test_custom_commands_rejects_empty_name(self) -> None: """Test that custom-commands with empty name fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1084,8 +1104,12 @@ def test_custom_commands_rejects_empty_name(self, schema: dict[str, Any]) -> Non with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_empty_description(self, schema: dict[str, Any]) -> None: + def test_custom_commands_rejects_empty_description(self) -> None: """Test that custom-commands with empty description fails validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") + with open(schema_path) as f: + schema = yaml.safe_load(f) + config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index 3fff6d87..ffbe9982 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -539,20 +539,6 @@ def test_skips_invalid_name_pattern(self, mock_webhook: Mock) -> None: assert len(result) == 1 assert result[0]["name"] == "valid-cmd" - def test_skips_duplicate_names(self, mock_webhook: Mock) -> None: - """Duplicate command names should be skipped with warning.""" - raw = [ - {"name": "deploy", "description": "First"}, - {"name": "deploy", "description": "Second"}, - {"name": "unique", "description": "Third"}, - ] - result = GithubWebhook._validate_custom_commands(mock_webhook, raw) - assert len(result) == 2 - assert result[0]["name"] == "deploy" - assert result[0]["description"] == "First" - assert result[1]["name"] == "unique" - mock_webhook.logger.warning.assert_any_call("[TEST] Custom command name 'deploy' is duplicated, skipping") - def test_all_invalid_returns_empty(self, mock_webhook: Mock) -> None: """When all entries are invalid, should return empty list.""" raw = ["not-a-dict", {"name": "bad name"}, {"description": "no name"}] From d25d6992d46c6551d3456cad124c95e7199eebdc Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 28 Jun 2026 12:29:59 +0300 Subject: [PATCH 24/25] fix: address review findings for custom commands feature - Add *, _, ~, <, > to _escape_markdown() for complete markdown/HTML injection prevention - Fix observability gap: log explicit warning when all custom commands are invalid - Make validation summary warnings mutually exclusive (if/elif) - Extend test coverage for new escape characters and all-invalid warning - Add Custom Commands documentation to README, config reference, and PR automation docs Co-authored-by: PI (claude-opus-4-6) Signed-off-by: rnetser Assisted-by: Claude --- CLAUDE.md | 4 +- README.md | 1 + docs/configuration-reference.md | 2 + docs/pull-request-automation.md | 27 ++++++++++ uv.lock | 2 + webhook_server/libs/github_api.py | 12 ++++- .../libs/handlers/pull_request_handler.py | 4 +- webhook_server/tests/test_config_schema.py | 50 +++++-------------- .../test_prepare_retest_welcome_comment.py | 26 +++++++++- 9 files changed, 85 insertions(+), 43 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a3dcb20a..e5936a0f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -703,7 +703,7 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Config:** Three resolution layers (first match wins, no list merge): (1) repo-local `.github-webhook-server.yaml`, (2) `repositories..custom-commands` in `config.yaml`, (3) root-level `custom-commands` in `config.yaml`. Per-repo layers can use an empty list (`custom-commands: []`) to disable global defaults; the root-level schema requires `minItems: 1`. -**Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`). Invalid entries are logged and skipped. +**Validation:** `GithubWebhook._validate_custom_commands()` in `webhook_server/libs/github_api.py` — validates at load time (entries must be dicts with non-empty `name` matching `^[a-zA-Z0-9_-]+$` and non-empty `description`; duplicate names are rejected, keeping only the first occurrence). Invalid and duplicate entries are logged and skipped. **Handler:** `PullRequestHandler._prepare_custom_commands_welcome_section` in `webhook_server/libs/handlers/pull_request_handler.py` — renders a "Custom Commands" section with each command as `` * `/{name}` - description ``. Descriptions are markdown-escaped via `_escape_markdown()`. @@ -726,7 +726,7 @@ User-defined commands rendered in the PR welcome message. Documentation-only — **Dual healthcheck:** Dockerfile `HEALTHCHECK` verifies both the webhook server (`:5000/webhook_server/healthcheck`) and the sidecar (`:${SIDECAR_PORT}/health`). Container is unhealthy if either fails. -**Docker build:** Multi-stage — `sidecar-builder` stage (node:26-slim) runs `npm ci`, `npx tsc`, `npm prune --omit=dev`. Final stage copies only `dist/`, `node_modules/` (production), and `package.json`. +**Docker build:** Multi-stage — `sidecar-builder` stage (node:22-slim) runs `npm ci`, `npx tsc`, `npm prune --omit=dev`. Final stage copies only `dist/`, `node_modules/` (production), and `package.json`. **Security note:** The AI conflict resolution prompt includes commit messages from the PR (user-controlled text). Prompt injection risk is mitigated by restricting the AI to read-only diff --git a/README.md b/README.md index 6164bf26..13d619aa 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ A [FastAPI](https://fastapi.tiangolo.com)-based webhook server for automating Gi - **OWNERS-Based Permissions** — reviewer and approver assignment from OWNERS files with per-directory granularity - **Container and PyPI Publishing** — automated container builds, tag-based releases, and PyPI publishing - **Issue Comment Commands** — `/retest`, `/approve`, `/cherry-pick`, `/build-and-push-container`, and more +- **Custom Commands** — user-defined documentation-only commands rendered in the PR welcome message - **AI Features** — conventional commit title validation and suggestions via Claude, Gemini, or Cursor - **Repository Bootstrap** — automatic label creation, branch protection, and webhook configuration on startup - **Log Viewer** — real-time log streaming, webhook flow visualization, and structured log analysis diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index 98c816d4..fe4b7784 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -299,6 +299,8 @@ The following keys are written under `repositories.` in `config - `custom-check-runs[].name`: Custom GitHub check-run name. It must be unique, use only safe characters, and not collide with built-in names such as `tox`, `pre-commit`, `build-container`, `python-module-install`, `conventional-title`, or `can-be-merged`. - `custom-check-runs[].command`: Command run in the repository worktree. Environment-variable prefixes and multiline commands are supported, but the executable must exist on the server. - `custom-check-runs[].mandatory`: Whether the custom check must pass for mergeability. Default is `true`. `false` checks still run; they just stop gating merges. +- `custom-commands[].name`: Name for a documentation-only command rendered in the PR welcome message. Must match `^[a-zA-Z0-9_-]+$`. Rendered as `/` in the welcome comment. +- `custom-commands[].description`: Human-readable description shown next to the command in the welcome comment. Markdown and HTML special characters are automatically escaped. - `test-oracle.server-url`, `test-oracle.ai-provider`, `test-oracle.ai-model`, `test-oracle.test-patterns`, `test-oracle.triggers`: Same meanings as the global `test-oracle` keys. A repository-level object replaces the global object for that repository. - `ai-features.ai-provider`, `ai-features.ai-model`, `ai-features.conventional-title.enabled`, `ai-features.conventional-title.mode`, `ai-features.conventional-title.timeout-minutes`, `ai-features.resolve-cherry-pick-conflicts-with-ai.enabled`, `ai-features.resolve-cherry-pick-conflicts-with-ai.timeout-minutes`: Same meanings as the global `ai-features` keys. A repository-level object replaces the global object for that repository. diff --git a/docs/pull-request-automation.md b/docs/pull-request-automation.md index c496e872..c818ce22 100644 --- a/docs/pull-request-automation.md +++ b/docs/pull-request-automation.md @@ -320,6 +320,33 @@ else: > **Tip:** If you rely on cherry-pick automation, keep the `cherry-pick` label category enabled and set `cherry-pick-assign-to-pr-author: true` if you want the follow-up PR to land on the original author by default. +## Custom Commands + +Custom commands let you document repository-specific workflows directly in the PR welcome comment. Unlike `custom-check-runs`, these are **documentation-only** — the server renders them in the welcome message but does not execute anything when they are invoked. + +Each command has a `name` and a `description`. Names must match the pattern `^[a-zA-Z0-9_-]+$` and are rendered as `/` in the welcome comment. Descriptions are escaped to prevent markdown injection. + +```yaml +# In config.yaml (global) or .github-webhook-server.yaml (per-repo) +custom-commands: + - name: run-tests + description: Run the full test suite locally before merging + - name: deploy-staging + description: Deploy this PR to the staging environment for review +``` + +The welcome comment renders a **Custom Commands** section: + +``` +#### Custom Commands +- `/run-tests` — Run the full test suite locally before merging +- `/deploy-staging` — Deploy this PR to the staging environment for review +``` + +**Validation:** Commands are validated at load time. Invalid entries (missing name/description, unsafe name characters, duplicates) are skipped with a warning. If all entries are invalid, a summary warning is logged. + +> **Tip:** Custom commands are reloaded on every webhook event — no server restart needed. Update the config file and use `/regenerate-welcome` to refresh the PR comment. + ## Key Configuration Most PR automation settings can be defined globally in `config.yaml`. Many of the same keys can also be overridden per repository in `.github-webhook-server.yaml`. diff --git a/uv.lock b/uv.lock index c9d5bfec..5ab2d64b 100644 --- a/uv.lock +++ b/uv.lock @@ -515,6 +515,7 @@ dev = [ { name = "types-requests" }, ] tests = [ + { name = "jsonschema" }, { name = "psutil" }, { name = "pytest-asyncio" }, { name = "pytest-xdist" }, @@ -564,6 +565,7 @@ dev = [ { name = "types-requests", specifier = ">=2.32.4.20250611" }, ] tests = [ + { name = "jsonschema", specifier = ">=4.0.0" }, { name = "psutil", specifier = ">=7.0.0" }, { name = "pytest-asyncio", specifier = ">=0.26.0" }, { name = "pytest-xdist", specifier = ">=3.7.0" }, diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index e2296f24..e138b5f2 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -1625,6 +1625,7 @@ def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str] prefix = getattr(self, "log_prefix", "") safe_name_pattern = re.compile(r"^[a-zA-Z0-9_-]+$") + seen_names: set[str] = set() validated: list[dict[str, str]] = [] for cmd in raw_commands: @@ -1646,11 +1647,20 @@ def _validate_custom_commands(self, raw_commands: object) -> list[dict[str, str] self.logger.warning(f"{prefix} Custom command name {name!r} does not match safe pattern, skipping") continue + if name in seen_names: + self.logger.warning(f"{prefix} Custom command name {name!r} is duplicated, skipping") + continue + seen_names.add(name) + validated.append({"name": name, "description": description}) if validated: self.logger.info(f"{prefix} Loaded {len(validated)} custom command(s): {[c['name'] for c in validated]}") - if validated and len(validated) < len(raw_commands): + if raw_commands and not validated: + self.logger.warning( + f"{prefix} No valid custom commands loaded — all {len(raw_commands)} entries were invalid" + ) + elif len(validated) < len(raw_commands): self.logger.warning(f"{prefix} Skipped {len(raw_commands) - len(validated)} invalid custom command(s)") return validated diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index 08d00ecc..c8821887 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -849,9 +849,9 @@ def _escape_markdown(text: str) -> str: Prevents markdown injection when inserting user-provided text into PR comments. Escapes characters that could create links, - images, or inline code. + images, inline code, bold, underline, strikethrough, or HTML tags. """ - for char in ("[", "]", "(", ")", "!", "`"): + for char in ("[", "]", "(", ")", "!", "`", "*", "_", "~", "<", ">"): text = text.replace(char, f"\\{char}") return text diff --git a/webhook_server/tests/test_config_schema.py b/webhook_server/tests/test_config_schema.py index cd9f6bf5..6080f760 100644 --- a/webhook_server/tests/test_config_schema.py +++ b/webhook_server/tests/test_config_schema.py @@ -1012,29 +1012,25 @@ def test_ai_features_resolve_cherry_pick_conflicts_with_ai_repository_level( class TestCustomCommandsSchema: """Tests for custom-commands schema validation.""" - def test_schema_allows_global_custom_commands(self) -> None: - """Test that schema.yaml defines custom-commands at root level.""" + @pytest.fixture + def schema(self) -> dict[str, Any]: schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") with open(schema_path) as f: - schema = yaml.safe_load(f) + return yaml.safe_load(f) + + def test_schema_allows_global_custom_commands(self, schema: dict[str, Any]) -> None: + """Test that schema.yaml defines custom-commands at root level.""" assert "custom-commands" in schema["properties"] assert schema["properties"]["custom-commands"]["type"] == "array" - def test_schema_allows_per_repo_custom_commands(self) -> None: + def test_schema_allows_per_repo_custom_commands(self, schema: dict[str, Any]) -> None: """Test that schema.yaml defines custom-commands in per-repo properties.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) repo_props = schema["properties"]["repositories"]["additionalProperties"]["properties"] assert "custom-commands" in repo_props assert repo_props["custom-commands"]["type"] == "array" - def test_custom_commands_valid_config(self) -> None: + def test_custom_commands_valid_config(self, schema: dict[str, Any]) -> None: """Test that valid custom-commands config passes schema validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1044,12 +1040,8 @@ def test_custom_commands_valid_config(self) -> None: } validate(instance=config, schema=schema) - def test_custom_commands_rejects_missing_name(self) -> None: + def test_custom_commands_rejects_missing_name(self, schema: dict[str, Any]) -> None: """Test that custom-commands without name fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1059,12 +1051,8 @@ def test_custom_commands_rejects_missing_name(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_invalid_name_pattern(self) -> None: + def test_custom_commands_rejects_invalid_name_pattern(self, schema: dict[str, Any]) -> None: """Test that custom-commands with invalid name pattern fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1074,12 +1062,8 @@ def test_custom_commands_rejects_invalid_name_pattern(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_extra_properties(self) -> None: + def test_custom_commands_rejects_extra_properties(self, schema: dict[str, Any]) -> None: """Test that custom-commands with extra properties fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1089,12 +1073,8 @@ def test_custom_commands_rejects_extra_properties(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_empty_name(self) -> None: + def test_custom_commands_rejects_empty_name(self, schema: dict[str, Any]) -> None: """Test that custom-commands with empty name fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ @@ -1104,12 +1084,8 @@ def test_custom_commands_rejects_empty_name(self) -> None: with pytest.raises(ValidationError): validate(instance=config, schema=schema) - def test_custom_commands_rejects_empty_description(self) -> None: + def test_custom_commands_rejects_empty_description(self, schema: dict[str, Any]) -> None: """Test that custom-commands with empty description fails validation.""" - schema_path = os.path.join(os.path.dirname(__file__), "..", "config", "schema.yaml") - with open(schema_path) as f: - schema = yaml.safe_load(f) - config: dict[str, Any] = { "github-tokens": ["ghp_test123"], # pragma: allowlist secret "custom-commands": [ diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index ffbe9982..7f6825f4 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -453,12 +453,19 @@ def test_custom_commands_escapes_markdown_in_description( """Markdown special characters in description should be escaped.""" commands = [ {"name": "deploy", "description": "Deploy [this](link) and `code` with !image"}, + {"name": "format", "description": "Make *bold* and _underline_ and ~~strike~~"}, + {"name": "inject", "description": "Try
and "}, ] handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) section = handler._prepare_custom_commands_welcome_section assert r"\[this\]\(link\)" in section assert r"\`code\`" in section assert r"\!image" in section + assert r"\*bold\*" in section + assert r"\_underline\_" in section + assert r"\~\~strike\~\~" in section + assert r"\" in section + assert r"\" in section class TestValidateCustomCommands: @@ -539,8 +546,25 @@ def test_skips_invalid_name_pattern(self, mock_webhook: Mock) -> None: assert len(result) == 1 assert result[0]["name"] == "valid-cmd" + def test_skips_duplicate_names(self, mock_webhook: Mock) -> None: + """Duplicate command names should be skipped with warning.""" + raw = [ + {"name": "deploy", "description": "First"}, + {"name": "deploy", "description": "Second"}, + {"name": "unique", "description": "Third"}, + ] + result = GithubWebhook._validate_custom_commands(mock_webhook, raw) + assert len(result) == 2 + assert result[0]["name"] == "deploy" + assert result[0]["description"] == "First" + assert result[1]["name"] == "unique" + mock_webhook.logger.warning.assert_any_call("[TEST] Custom command name 'deploy' is duplicated, skipping") + def test_all_invalid_returns_empty(self, mock_webhook: Mock) -> None: - """When all entries are invalid, should return empty list.""" + """When all entries are invalid, should return empty list and log all-invalid warning.""" raw = ["not-a-dict", {"name": "bad name"}, {"description": "no name"}] result = GithubWebhook._validate_custom_commands(mock_webhook, raw) assert result == [] + mock_webhook.logger.warning.assert_any_call( + f"[TEST] No valid custom commands loaded \u2014 all {len(raw)} entries were invalid" + ) From feb76b690bcc64d9261639581b25128aa3947812 Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 28 Jun 2026 12:41:12 +0300 Subject: [PATCH 25/25] fix: neutralize @mentions and precise docs escaping claims - Add @ mention neutralization in _escape_markdown() using zero-width space - Update docs to precisely list escaped/neutralized characters - Add test for @org/team mention neutralization Co-authored-by: PI (claude-opus-4-6) Signed-off-by: rnetser Assisted-by: Claude --- docs/configuration-reference.md | 2 +- webhook_server/libs/handlers/pull_request_handler.py | 5 ++++- webhook_server/tests/test_prepare_retest_welcome_comment.py | 2 ++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index fe4b7784..08e020d3 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -300,7 +300,7 @@ The following keys are written under `repositories.` in `config - `custom-check-runs[].command`: Command run in the repository worktree. Environment-variable prefixes and multiline commands are supported, but the executable must exist on the server. - `custom-check-runs[].mandatory`: Whether the custom check must pass for mergeability. Default is `true`. `false` checks still run; they just stop gating merges. - `custom-commands[].name`: Name for a documentation-only command rendered in the PR welcome message. Must match `^[a-zA-Z0-9_-]+$`. Rendered as `/` in the welcome comment. -- `custom-commands[].description`: Human-readable description shown next to the command in the welcome comment. Markdown and HTML special characters are automatically escaped. +- `custom-commands[].description`: Human-readable description shown next to the command in the welcome comment. Markdown formatting characters (`*`, `_`, `~`, `` ` ``), link/image syntax (`[`, `]`, `(`, `)`, `!`), HTML angle brackets (`<`, `>`), and `@` mentions are automatically neutralized. - `test-oracle.server-url`, `test-oracle.ai-provider`, `test-oracle.ai-model`, `test-oracle.test-patterns`, `test-oracle.triggers`: Same meanings as the global `test-oracle` keys. A repository-level object replaces the global object for that repository. - `ai-features.ai-provider`, `ai-features.ai-model`, `ai-features.conventional-title.enabled`, `ai-features.conventional-title.mode`, `ai-features.conventional-title.timeout-minutes`, `ai-features.resolve-cherry-pick-conflicts-with-ai.enabled`, `ai-features.resolve-cherry-pick-conflicts-with-ai.timeout-minutes`: Same meanings as the global `ai-features` keys. A repository-level object replaces the global object for that repository. diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index c8821887..b8b64573 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -849,10 +849,13 @@ def _escape_markdown(text: str) -> str: Prevents markdown injection when inserting user-provided text into PR comments. Escapes characters that could create links, - images, inline code, bold, underline, strikethrough, or HTML tags. + images, inline code, bold, underline, strikethrough, or HTML tags, + and neutralizes @mentions. """ for char in ("[", "]", "(", ")", "!", "`", "*", "_", "~", "<", ">"): text = text.replace(char, f"\\{char}") + # Neutralize @mentions to prevent unintended user/team pings + text = text.replace("@", "@\u200b") return text @property diff --git a/webhook_server/tests/test_prepare_retest_welcome_comment.py b/webhook_server/tests/test_prepare_retest_welcome_comment.py index 7f6825f4..551b8c58 100644 --- a/webhook_server/tests/test_prepare_retest_welcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_welcome_comment.py @@ -455,6 +455,7 @@ def test_custom_commands_escapes_markdown_in_description( {"name": "deploy", "description": "Deploy [this](link) and `code` with !image"}, {"name": "format", "description": "Make *bold* and _underline_ and ~~strike~~"}, {"name": "inject", "description": "Try
and "}, + {"name": "mention", "description": "Contact @org/team for help"}, ] handler = self._create_handler(process_github_webhook, owners_file_handler, custom_commands=commands) section = handler._prepare_custom_commands_welcome_section @@ -466,6 +467,7 @@ def test_custom_commands_escapes_markdown_in_description( assert r"\~\~strike\~\~" in section assert r"\" in section assert r"\" in section + assert "@\u200borg/team" in section class TestValidateCustomCommands: