From 2470c3be6c0aa1b7f1e8160ebe0e5d2244f713fc Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Thu, 12 Mar 2026 17:49:35 +0200 Subject: [PATCH 1/2] refactor: migrate conventional-title config from string to dict (#1027) Change ai-features.conventional-title from string enum (true/false/fix) to dict with enabled, mode (suggest/fix), and timeout-minutes. Config: ai-features: conventional-title: enabled: true mode: suggest # suggest | fix timeout-minutes: 10 Signed-off-by: Meni Yakove --- README.md | 16 +++++---- examples/.github-webhook-server.yaml | 5 ++- examples/config.yaml | 5 ++- webhook_server/config/schema.yaml | 35 +++++++++++++------ .../libs/handlers/runner_handler.py | 20 ++++++----- webhook_server/tests/test_runner_handler.py | 14 ++++---- 6 files changed, 62 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index cee16a0a..c89d18b0 100644 --- a/README.md +++ b/README.md @@ -1258,14 +1258,18 @@ AI-suggested fixes for PR titles that don't follow the Conventional Commits form ai-features: ai-provider: "claude" ai-model: "claude-opus-4-6[1m]" - conventional-title: "true" # "true": suggest in check run | "false": disabled | "fix": auto-update PR title + conventional-title: + enabled: true + mode: suggest # suggest | fix + timeout-minutes: 10 # default: 10 ``` -| Mode | Behavior | -|------|----------| -| `"true"` | Shows AI-suggested title in check run output when validation fails | -| `"false"` | Disabled (default) | -| `"fix"` | Automatically updates the PR title with the AI suggestion | +| Setting | Values | Description | +|---------|--------|-------------| +| `enabled` | `true` / `false` | Enable or disable AI conventional title suggestions | +| `mode` | `suggest` (default) | Shows AI-suggested title in check run output when validation fails | +| | `fix` | Automatically updates the PR title with the AI suggestion | +| `timeout-minutes` | integer (default: `10`) | Timeout in minutes for the AI CLI process | ### Cherry-Pick Conflict Resolution diff --git a/examples/.github-webhook-server.yaml b/examples/.github-webhook-server.yaml index 658098fd..43b2519f 100644 --- a/examples/.github-webhook-server.yaml +++ b/examples/.github-webhook-server.yaml @@ -151,7 +151,10 @@ pr-size-thresholds: ai-features: ai-provider: "claude" # claude | gemini | cursor ai-model: "claude-opus-4-6[1m]" - conventional-title: "true" # "true": suggest in checkrun | "false": disabled | "fix": auto-update PR title + conventional-title: + enabled: true + mode: suggest # suggest: show in checkrun | fix: auto-update PR title + timeout-minutes: 10 resolve-cherry-pick-conflicts-with-ai: enabled: true timeout-minutes: 10 # Timeout in minutes for AI CLI (default: 10) diff --git a/examples/config.yaml b/examples/config.yaml index 171f77a7..26c6ce45 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -128,7 +128,10 @@ test-oracle: ai-features: ai-provider: "claude" # claude | gemini | cursor ai-model: "claude-opus-4-6[1m]" - conventional-title: "true" # "true": suggest in checkrun | "false": disabled | "fix": auto-update PR title + conventional-title: + enabled: true + mode: suggest # suggest: show in checkrun | fix: auto-update PR title + timeout-minutes: 10 resolve-cherry-pick-conflicts-with-ai: enabled: true timeout-minutes: 10 # Timeout in minutes for AI CLI (default: 10) diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 1c09ca64..78565127 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -19,17 +19,32 @@ $defs: type: string description: AI model identifier (e.g., claude-opus-4-6[1m], sonnet, gemini-2.5-pro) conventional-title: - type: string - enum: - - "true" - - "false" - - "fix" + type: object description: | - AI-powered conventional title suggestions. - - "true": Show AI-suggested title in check run output when validation fails - - "false": Disabled (default) - - "fix": Auto-fix PR title with AI suggestion when validation fails - default: "false" + AI-powered conventional title configuration. + When enabled, suggests or auto-fixes PR titles that don't follow + Conventional Commits format. + properties: + enabled: + type: boolean + description: Enable AI conventional title suggestions + mode: + type: string + enum: + - suggest + - fix + description: | + - suggest: Show AI-suggested title in check run output when validation fails + - fix: Auto-fix PR title with AI suggestion when validation fails + default: suggest + timeout-minutes: + type: integer + minimum: 1 + description: Timeout in minutes for the AI CLI process (default 10) + default: 10 + required: + - enabled + additionalProperties: false resolve-cherry-pick-conflicts-with-ai: type: object description: | diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index 46ffa804..dd72c2f6 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -569,7 +569,7 @@ async def run_conventional_title_check(self, pull_request: PullRequest) -> None: f"Suggestion was invalid or unchanged." ) - elif ai_suggestion and ai_mode == "true" and output["text"] is not None: + elif ai_suggestion and ai_mode == "suggest" and output["text"] is not None: output["text"] += f"\n\n---\n\n### AI-Suggested Title\n\n> {ai_suggestion}\n" await self.check_run_handler.set_check_failure(name=CONVENTIONAL_TITLE_STR, output=output) @@ -578,18 +578,17 @@ def _get_ai_conventional_title_mode(self) -> str | None: """Get the conventional-title AI mode from config. Returns: - "true" for suggestion mode, "fix" for auto-fix mode, or None if disabled. + "suggest" for suggestion mode, "fix" for auto-fix mode, or None if disabled. """ ai_config = self.github_webhook.ai_features if not ai_config: return None - ct_value = ai_config.get("conventional-title", "false") - if ct_value == "true": - return "true" - if ct_value == "fix": - return "fix" - return None + ct_config = ai_config.get("conventional-title") + if not isinstance(ct_config, dict) or not ct_config.get("enabled"): + return None + + return ct_config.get("mode", "suggest") async def _get_ai_title_suggestion(self, title: str, allowed_names: list[str], *, is_wildcard: bool) -> str | None: """Get an AI-suggested conventional title when validation fails. @@ -606,6 +605,10 @@ async def _get_ai_title_suggestion(self, title: str, allowed_names: list[str], * ai_provider, ai_model = ai_result + ai_config = self.github_webhook.ai_features + ct_config = ai_config.get("conventional-title", {}) if ai_config else {} + timeout_minutes = ct_config.get("timeout-minutes", 10) if isinstance(ct_config, dict) else 10 + if is_wildcard: types_info = "Any type name is accepted (wildcard mode)." else: @@ -638,6 +641,7 @@ async def _get_ai_title_suggestion(self, title: str, allowed_names: list[str], * ai_model=ai_model, cwd=self.github_webhook.clone_repo_dir, cli_flags=cli_flags, + timeout_minutes=timeout_minutes, ) if success: diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index b01cb7a8..4dc9f2a8 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -709,7 +709,7 @@ async def test_conventional_title_failure_with_ai_suggestion( runner_handler.github_webhook.ai_features = { "ai-provider": "claude", "ai-model": "sonnet", - "conventional-title": "true", + "conventional-title": {"enabled": True, "mode": "suggest"}, } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" @@ -771,7 +771,7 @@ async def test_conventional_title_fix_mode_updates_pr_title( runner_handler.github_webhook.ai_features = { "ai-provider": "claude", "ai-model": "sonnet", - "conventional-title": "fix", + "conventional-title": {"enabled": True, "mode": "fix"}, } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" @@ -808,7 +808,7 @@ async def test_conventional_title_fix_mode_edit_failure( runner_handler.github_webhook.ai_features = { "ai-provider": "claude", "ai-model": "sonnet", - "conventional-title": "fix", + "conventional-title": {"enabled": True, "mode": "fix"}, } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" @@ -845,13 +845,13 @@ async def mock_to_thread_side_effect(func: Any, *args: Any, **kwargs: Any) -> An async def test_conventional_title_disabled_mode( self, runner_handler: RunnerHandler, mock_pull_request: Mock ) -> None: - """Test that conventional-title: "false" disables AI suggestion.""" + """Test that conventional-title disabled disables AI suggestion.""" runner_handler.github_webhook.conventional_title = "feat,fix" mock_pull_request.title = "bad title" runner_handler.github_webhook.ai_features = { "ai-provider": "claude", "ai-model": "sonnet", - "conventional-title": "false", + "conventional-title": {"enabled": False}, } with patch.object( @@ -883,7 +883,7 @@ async def test_conventional_title_ai_cli_failure( runner_handler.github_webhook.ai_features = { "ai-provider": "claude", "ai-model": "sonnet", - "conventional-title": "true", + "conventional-title": {"enabled": True, "mode": "suggest"}, } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" @@ -915,7 +915,7 @@ async def test_conventional_title_ai_cli_exception( runner_handler.github_webhook.ai_features = { "ai-provider": "claude", "ai-model": "sonnet", - "conventional-title": "true", + "conventional-title": {"enabled": True, "mode": "suggest"}, } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" From 984c3321ffae81a79dfa457b229bf540047e8248 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Thu, 12 Mar 2026 18:45:43 +0200 Subject: [PATCH 2/2] fix: validate mode value and set check success after AI fix (#1027) - Validate conventional-title mode against (suggest, fix) with fallback - Set check to success after valid AI title auto-fix - Run AI title suggestion in PR worktree for correct context - Assert timeout_minutes in test Signed-off-by: Meni Yakove --- .../libs/handlers/runner_handler.py | 71 +++++++----- webhook_server/tests/test_runner_handler.py | 109 ++++++++++++------ 2 files changed, 115 insertions(+), 65 deletions(-) diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py index dd72c2f6..c2fef8d6 100644 --- a/webhook_server/libs/handlers/runner_handler.py +++ b/webhook_server/libs/handlers/runner_handler.py @@ -524,6 +524,7 @@ async def run_conventional_title_check(self, pull_request: PullRequest) -> None: """ # AI-suggested title (if ai-features configured) ai_suggestion = await self._get_ai_title_suggestion( + pull_request=pull_request, title=title, allowed_names=allowed_names, is_wildcard=is_wildcard, @@ -544,12 +545,16 @@ async def run_conventional_title_check(self, pull_request: PullRequest) -> None: self.logger.info(f"{self.log_prefix} AI fixing PR title from '{title}' to '{ai_suggestion}'") try: await asyncio.to_thread(pull_request.edit, title=ai_suggestion) - if output["text"] is not None: - output["text"] += ( - f"\n\n---\n\n### AI Auto-Fix\n\n" - f"Title updated to: `{ai_suggestion}`\n" - f"Check will re-run automatically." - ) + output["title"] = "Conventional Title" + output["summary"] = "PR title auto-fixed by AI" + output["text"] = ( + f"**AI Auto-Fix Applied**\n\n" + f"Title updated from: `{title}`\n" + f"Title updated to: `{ai_suggestion}`\n" + ) + return await self.check_run_handler.set_check_success( + name=CONVENTIONAL_TITLE_STR, output=output + ) except Exception: self.logger.exception(f"{self.log_prefix} Failed to auto-fix PR title") if output["text"] is not None: @@ -588,9 +593,15 @@ def _get_ai_conventional_title_mode(self) -> str | None: if not isinstance(ct_config, dict) or not ct_config.get("enabled"): return None - return ct_config.get("mode", "suggest") + mode = ct_config.get("mode", "suggest") + if mode not in ("suggest", "fix"): + self.logger.warning(f"{self.log_prefix} Invalid conventional-title mode '{mode}', defaulting to 'suggest'") + return "suggest" + return mode - async def _get_ai_title_suggestion(self, title: str, allowed_names: list[str], *, is_wildcard: bool) -> str | None: + async def _get_ai_title_suggestion( + self, pull_request: PullRequest, title: str, allowed_names: list[str], *, is_wildcard: bool + ) -> str | None: """Get an AI-suggested conventional title when validation fails. Returns the suggestion string or None if AI features are not configured or on error. @@ -614,16 +625,15 @@ async def _get_ai_title_suggestion(self, title: str, allowed_names: list[str], * else: types_info = f"Allowed types: {', '.join(allowed_names)}" - # The repository clone is already checked out to the PR branch - # (done by _clone_repository in github_api.py), so the AI CLI has - # full access to the PR's commits and changes from the cwd. prompt = ( "You are in a git repository checked out to a PR branch.\n" "Look at the recent commits and changes to understand what this PR does.\n" f"Current PR title: {title}\n" f"{types_info}\n" - f"Format: [optional scope]: \n" - "Reply with ONLY the suggested title, nothing else." + f"Required format: [optional scope]: \n" + f"Output ONLY the corrected title on a single line.\n" + f"Do NOT include any explanation, reasoning, markdown, or quotes.\n" + f"Example output: feat: add user authentication" ) cli_flags: list[str] = [] @@ -635,23 +645,28 @@ async def _get_ai_title_suggestion(self, title: str, allowed_names: list[str], * cli_flags = ["--force"] try: - success, result = await call_ai_cli( - prompt=prompt, - ai_provider=ai_provider, - ai_model=ai_model, - cwd=self.github_webhook.clone_repo_dir, - cli_flags=cli_flags, - timeout_minutes=timeout_minutes, - ) + async with self._checkout_worktree(pull_request=pull_request) as (wt_success, worktree_path, _, _): + if not wt_success: + self.logger.warning(f"{self.log_prefix} Failed to create worktree for AI title suggestion") + return None + + success, result = await call_ai_cli( + prompt=prompt, + ai_provider=ai_provider, + ai_model=ai_model, + cwd=worktree_path, + cli_flags=cli_flags, + timeout_minutes=timeout_minutes, + ) - if success: - # Clean up the response - take first line, strip backticks/quotes - suggestion = result.strip().splitlines()[0].strip().strip("`").strip('"').strip("'") - self.logger.info(f"{self.log_prefix} AI suggested title: {suggestion}") - return suggestion + if success: + # Clean up the response - take first line, strip backticks/quotes + suggestion = result.strip().splitlines()[0].strip().strip("`").strip('"').strip("'") + self.logger.info(f"{self.log_prefix} AI suggested title: {suggestion}") + return suggestion - self.logger.warning(f"{self.log_prefix} AI title suggestion failed: {result}") - return None + self.logger.warning(f"{self.log_prefix} AI title suggestion failed: {result}") + return None except Exception: self.logger.exception(f"{self.log_prefix} AI title suggestion failed unexpectedly") diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index 4dc9f2a8..822e5781 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -713,6 +713,12 @@ async def test_conventional_title_failure_with_ai_suggestion( } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" + worktree_path = "/tmp/test-clone-worktree-abc123" + + @asynccontextmanager + async def mock_checkout_worktree(**kwargs: Any) -> AsyncGenerator[tuple[bool, str, str, str]]: + yield (True, worktree_path, "", "") + with patch.object( runner_handler.check_run_handler, "is_check_run_in_progress", new=AsyncMock(return_value=False) ): @@ -726,17 +732,19 @@ async def test_conventional_title_failure_with_ai_suggestion( new_callable=AsyncMock, return_value=(True, "fix: correct the bad title"), ) as mock_ai_cli: - await runner_handler.run_conventional_title_check(mock_pull_request) + with patch.object(runner_handler, "_checkout_worktree", side_effect=mock_checkout_worktree): + await runner_handler.run_conventional_title_check(mock_pull_request) - mock_set_failure.assert_awaited_once() - output = mock_set_failure.call_args[1]["output"] - assert "AI-Suggested Title" in output["text"] - assert "fix: correct the bad title" in output["text"] + mock_set_failure.assert_awaited_once() + output = mock_set_failure.call_args[1]["output"] + assert "AI-Suggested Title" in output["text"] + assert "fix: correct the bad title" in output["text"] - # Verify cwd was passed to call_ai_cli as str - mock_ai_cli.assert_awaited_once() - call_kwargs = mock_ai_cli.call_args[1] - assert call_kwargs["cwd"] == "/tmp/test-clone" + # Verify cwd was passed to call_ai_cli as worktree path + mock_ai_cli.assert_awaited_once() + call_kwargs = mock_ai_cli.call_args[1] + assert call_kwargs["cwd"] == worktree_path + assert call_kwargs["timeout_minutes"] == 10 @pytest.mark.asyncio async def test_conventional_title_failure_without_ai_features( @@ -775,11 +783,17 @@ async def test_conventional_title_fix_mode_updates_pr_title( } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" + @asynccontextmanager + async def mock_checkout_worktree(**kwargs: Any) -> AsyncGenerator[tuple[bool, str, str, str]]: + yield (True, "/tmp/test-clone-worktree-abc123", "", "") + with patch.object( runner_handler.check_run_handler, "is_check_run_in_progress", new=AsyncMock(return_value=False) ): with patch.object(runner_handler.check_run_handler, "set_check_in_progress", new=AsyncMock()): - with patch.object(runner_handler.check_run_handler, "set_check_success", new=AsyncMock()): + with patch.object( + runner_handler.check_run_handler, "set_check_success", new=AsyncMock() + ) as mock_set_success: with patch.object( runner_handler.check_run_handler, "set_check_failure", new=AsyncMock() ) as mock_set_failure: @@ -788,15 +802,19 @@ async def test_conventional_title_fix_mode_updates_pr_title( new_callable=AsyncMock, return_value=(True, "fix: correct the title"), ): - with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: - await runner_handler.run_conventional_title_check(mock_pull_request) + with patch.object(runner_handler, "_checkout_worktree", side_effect=mock_checkout_worktree): + with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: + await runner_handler.run_conventional_title_check(mock_pull_request) - # Verify PR title was updated - mock_to_thread.assert_any_call(mock_pull_request.edit, title="fix: correct the title") - # Verify check was set to failure (will re-run automatically on title change) - mock_set_failure.assert_awaited_once() - output = mock_set_failure.call_args[1]["output"] - assert "AI Auto-Fix" in output["text"] + # Verify PR title was updated + mock_to_thread.assert_any_call( + mock_pull_request.edit, title="fix: correct the title" + ) + # Verify check was set to success after auto-fix + mock_set_success.assert_awaited_once() + mock_set_failure.assert_not_awaited() + output = mock_set_success.call_args[1]["output"] + assert "AI Auto-Fix Applied" in output["text"] @pytest.mark.asyncio async def test_conventional_title_fix_mode_edit_failure( @@ -812,6 +830,10 @@ async def test_conventional_title_fix_mode_edit_failure( } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" + @asynccontextmanager + async def mock_checkout_worktree(**kwargs: Any) -> AsyncGenerator[tuple[bool, str, str, str]]: + yield (True, "/tmp/test-clone-worktree-abc123", "", "") + with patch.object( runner_handler.check_run_handler, "is_check_run_in_progress", new=AsyncMock(return_value=False) ): @@ -825,21 +847,24 @@ async def test_conventional_title_fix_mode_edit_failure( new_callable=AsyncMock, return_value=(True, "fix: correct the title"), ): - # Make edit raise an exception - async def mock_to_thread_side_effect(func: Any, *args: Any, **kwargs: Any) -> Any: - if func == mock_pull_request.edit: - raise Exception("GitHub API error") - return func(*args, **kwargs) + with patch.object(runner_handler, "_checkout_worktree", side_effect=mock_checkout_worktree): + # Make edit raise an exception + async def mock_to_thread_side_effect(func: Any, *args: Any, **kwargs: Any) -> Any: + if func == mock_pull_request.edit: + raise Exception("GitHub API error") + return func(*args, **kwargs) - with patch( - "asyncio.to_thread", new_callable=AsyncMock, side_effect=mock_to_thread_side_effect - ): - await runner_handler.run_conventional_title_check(mock_pull_request) + with patch( + "asyncio.to_thread", + new_callable=AsyncMock, + side_effect=mock_to_thread_side_effect, + ): + await runner_handler.run_conventional_title_check(mock_pull_request) - # Check should still be set to failure - mock_set_failure.assert_awaited_once() - output = mock_set_failure.call_args[1]["output"] - assert "AI Auto-Fix Failed" in output["text"] + # Check should still be set to failure + mock_set_failure.assert_awaited_once() + output = mock_set_failure.call_args[1]["output"] + assert "AI Auto-Fix Failed" in output["text"] @pytest.mark.asyncio async def test_conventional_title_disabled_mode( @@ -887,6 +912,10 @@ async def test_conventional_title_ai_cli_failure( } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" + @asynccontextmanager + async def mock_checkout_worktree(**kwargs: Any) -> AsyncGenerator[tuple[bool, str, str, str]]: + yield (True, "/tmp/test-clone-worktree-abc123", "", "") + with patch.object( runner_handler.check_run_handler, "is_check_run_in_progress", new=AsyncMock(return_value=False) ): @@ -900,10 +929,11 @@ async def test_conventional_title_ai_cli_failure( new_callable=AsyncMock, return_value=(False, "CLI timeout"), ): - await runner_handler.run_conventional_title_check(mock_pull_request) - mock_set_failure.assert_awaited_once() - output = mock_set_failure.call_args[1]["output"] - assert "AI-Suggested Title" not in output["text"] + with patch.object(runner_handler, "_checkout_worktree", side_effect=mock_checkout_worktree): + await runner_handler.run_conventional_title_check(mock_pull_request) + mock_set_failure.assert_awaited_once() + output = mock_set_failure.call_args[1]["output"] + assert "AI-Suggested Title" not in output["text"] @pytest.mark.asyncio async def test_conventional_title_ai_cli_exception( @@ -919,6 +949,10 @@ async def test_conventional_title_ai_cli_exception( } runner_handler.github_webhook.clone_repo_dir = "/tmp/test-clone" + @asynccontextmanager + async def mock_checkout_worktree(**kwargs: Any) -> AsyncGenerator[tuple[bool, str, str, str]]: + yield (True, "/tmp/test-clone-worktree-abc123", "", "") + with patch.object( runner_handler.check_run_handler, "is_check_run_in_progress", new=AsyncMock(return_value=False) ): @@ -932,8 +966,9 @@ async def test_conventional_title_ai_cli_exception( new_callable=AsyncMock, side_effect=RuntimeError("boom"), ): - await runner_handler.run_conventional_title_check(mock_pull_request) - mock_set_failure.assert_awaited_once() + with patch.object(runner_handler, "_checkout_worktree", side_effect=mock_checkout_worktree): + await runner_handler.run_conventional_title_check(mock_pull_request) + mock_set_failure.assert_awaited_once() @pytest.mark.asyncio async def test_is_branch_exists(self, runner_handler: RunnerHandler) -> None: