Handle missing file path in PR title gracefully

services/webhook/check_suite_handler.py

@@ -582,7 +582,8 @@ async def handle_check_suite(
     for file_change in changed_files:
         allowed_to_edit_files.add(file_change["filename"])
     impl_file_path = get_impl_file_from_pr_title(pr_title)
-    allowed_to_edit_files.add(impl_file_path)
+    if impl_file_path:
+        allowed_to_edit_files.add(impl_file_path)
 
     p += 5
     add_log_message("Checked out the error log from the workflow run.", log_messages)

services/webhook/new_pr_handler.py

@@ -305,6 +305,9 @@ async def handle_new_pr(
 
     # Extract target implementation file
     impl_file_path = get_impl_file_from_pr_title(pr_title)
+    if not impl_file_path:
+        raise ValueError(f"No file path found in PR title: {pr_title}")
+
     if pr_title.startswith(SCHEDULE_PREFIX_INCREASE):
         base_args["impl_file_to_collect_coverage_from"] = impl_file_path
     test_dir_prefixes = repo_settings["test_dir_prefixes"] if repo_settings else []

services/webhook/test_successful_check_suite_handler.py

@@ -40,7 +40,9 @@ def test_handle_successful_check_suite_with_pr(
     mock_get_check_suites.return_value = [
         {"app": {"name": "CircleCI Checks"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
 
     with patch(
         "services.webhook.successful_check_suite_handler.supabase"
@@ -141,7 +143,9 @@ def test_handle_successful_check_suite_no_usage_record_found(
     mock_get_check_suites.return_value = [
         {"app": {"name": "GitHub Actions"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["GitHub Actions"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["GitHub Actions"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "clean"}
     mock_check_skip_ci.return_value = False
     mock_get_files.return_value = []
@@ -239,7 +243,9 @@ def test_auto_merge_success(
     mock_get_check_suites.return_value = [
         {"app": {"name": "CircleCI Checks"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "clean"}
     mock_check_skip_ci.return_value = False
     mock_get_files.return_value = [
@@ -368,7 +374,9 @@ def test_auto_merge_multiple_test_files_changed(
     mock_get_check_suites.return_value = [
         {"app": {"name": "CircleCI Checks"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "clean"}
     mock_check_skip_ci.return_value = False
     mock_get_files.return_value = [
@@ -520,7 +528,9 @@ def test_auto_merge_with_non_test_files_allowed(
     mock_get_check_suites.return_value = [
         {"app": {"name": "CircleCI Checks"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "clean"}
     mock_check_skip_ci.return_value = False
     mock_get_files.return_value = [
@@ -657,7 +667,9 @@ def test_auto_merge_blocked_skips_notification_when_checks_in_progress(
         },
         {"app": {"name": "Cypress"}, "status": "in_progress", "conclusion": None},
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "blocked"}
 
     with patch(
@@ -726,7 +738,9 @@ def test_auto_merge_with_blocked_state(
     mock_get_check_suites.return_value = [
         {"app": {"name": "CircleCI Checks"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "blocked"}
 
     with patch(
@@ -796,7 +810,9 @@ def test_auto_merge_with_unstable_state(
     mock_get_check_suites.return_value = [
         {"app": {"name": "CircleCI Checks"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "unstable"}
     mock_check_skip_ci.return_value = False
     mock_get_files.return_value = [
@@ -875,7 +891,9 @@ def test_auto_merge_with_unknown_state_no_comment(
     mock_get_check_suites.return_value = [
         {"app": {"name": "CircleCI Checks"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["CircleCI Checks"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["CircleCI Checks"], strict=True
+    )
     mock_get_pr.return_value = {"mergeable_state": "unknown"}
 
     with patch(
@@ -935,7 +953,9 @@ def test_skip_ci_returns_early(
     mock_get_check_suites.return_value = [
         {"app": {"name": "GitHub Actions"}, "status": "completed"}
     ]
-    mock_get_required_checks.return_value = StatusChecksResult(status_code=200, checks=["GitHub Actions"], strict=True)
+    mock_get_required_checks.return_value = StatusChecksResult(
+        status_code=200, checks=["GitHub Actions"], strict=True
+    )
     mock_check_skip_ci.return_value = True
 
     with patch(

utils/files/get_impl_file_from_pr_title.py

@@ -1,13 +1,16 @@
 from utils.error.handle_exceptions import handle_exceptions
+from utils.logging.logging_config import logger
 
 
-@handle_exceptions(raise_on_error=True)
+@handle_exceptions(default_return_value=None, raise_on_error=False)
 def get_impl_file_from_pr_title(pr_title: str):
     if not pr_title:
-        raise ValueError("pr_title is empty")
+        logger.warning("get_impl_file_from_pr_title: pr_title is empty")
+        return None
 
     last_token = pr_title.split()[-1]
     if "/" in last_token or "." in last_token:
         return last_token
 
-    raise ValueError(f"No file path found in PR title: {pr_title}")
+    logger.warning("No file path found in PR title: %s", pr_title)
+    return None

utils/files/is_target_test_file.py

@@ -4,20 +4,36 @@
 from utils.error.handle_exceptions import handle_exceptions
 from utils.files.get_impl_file_from_pr_title import get_impl_file_from_pr_title
 from utils.files.is_test_file import is_test_file
+from utils.logging.logging_config import logger
 
 
 @handle_exceptions(default_return_value=True, raise_on_error=False)
 def is_target_test_file(file_path: str, base_args: BaseArgs):
     if not is_test_file(file_path):
-        # Return False (block edit) when file is not a test file
+        logger.info(
+            "is_target_test_file: %s is not a test file, blocking edit", file_path
+        )
         return False
 
     pr_title = base_args.get("pr_title", "")
     if not pr_title:
-        # Return True (allow edit) when title is missing to avoid blocking GitAuto
+        logger.info("is_target_test_file: no pr_title, allowing edit for %s", file_path)
         return True
 
     implementation_file = get_impl_file_from_pr_title(pr_title)
-    target_filename = Path(implementation_file).stem
+    if not implementation_file:
+        logger.info(
+            "is_target_test_file: no file path in title, allowing edit for %s",
+            file_path,
+        )
+        return True
 
-    return target_filename.lower() in file_path.lower()
+    target_filename = Path(implementation_file).stem
+    is_target = target_filename.lower() in file_path.lower()
+    logger.info(
+        "is_target_test_file: %s, target=%s, file_path=%s",
+        is_target,
+        target_filename,
+        file_path,
+    )
+    return is_target

utils/files/test_get_impl_file_from_pr_title.py

@@ -1,5 +1,3 @@
-import pytest
-
 from utils.files.get_impl_file_from_pr_title import get_impl_file_from_pr_title
 
 
@@ -15,13 +13,11 @@ def test_achieve_coverage_prefix():
 
 def test_non_matching_title():
     title = "Fix bug in authentication"
-    with pytest.raises(ValueError):
-        get_impl_file_from_pr_title(title)
+    assert get_impl_file_from_pr_title(title) is None
 
 
 def test_empty_title():
-    with pytest.raises(ValueError):
-        get_impl_file_from_pr_title("")
+    assert get_impl_file_from_pr_title("") is None
 
 
 def test_file_at_root():
@@ -31,5 +27,4 @@ def test_file_at_root():
 
 def test_no_file_extension():
     title = "Fix the Makefile"
-    with pytest.raises(ValueError):
-        get_impl_file_from_pr_title(title)
+    assert get_impl_file_from_pr_title(title) is None