Combine API access check and folder listing into single request; close stale Jules PRs

WARP.md

@@ -91,7 +91,8 @@ The entire tool lives in `main.py` and is structured into clear phases:
    - `sanitize_for_log()` – Redacts `TOKEN` values from any log messages.
 
 4. **Control D API helpers**
-   - `list_existing_folders()` – Returns a `{folder_name -> folder_id}` mapping for a profile.
+   - `verify_access_and_get_folders()` – Combines the API access check and fetching existing folders into a single request. Returns `{folder_name -> folder_id}` on success.
+   - `list_existing_folders()` – Helper that returns a `{folder_name -> folder_id}` mapping (used as fallback).
    - `get_all_existing_rules()` – Collects all existing rule PKs from both the root and each folder, using a `ThreadPoolExecutor` to parallelize per-folder fetches while accumulating into a shared `set` guarded by a lock.
    - `delete_folder()` – Deletes a folder by ID with error-logged failures.
    - `create_folder()` – Creates a folder and tries to read its ID directly from the response; if that fails, it polls `GET /groups` with increasing waits (using `FOLDER_CREATION_DELAY`) until the new folder appears.
@@ -111,7 +112,8 @@ The entire tool lives in `main.py` and is structured into clear phases:
      2. Builds a `plan_entry` summarizing folder names, rule counts, and per-action breakdown (for `rule_groups`), appending it to the shared `plan_accumulator`.
      3. If `dry_run=True`, stops here after logging a summary message.
      4. Otherwise, reuses a single `_api_client()` instance to:
-        - List and optionally delete existing folders with matching names (`--no-delete` skips this step).
+        - Verify access and list existing folders in one request (`verify_access_and_get_folders`).
+        - Optionally delete existing folders with matching names (`--no-delete` skips this step).
         - If any deletions occurred, waits ~60 seconds (`countdown_timer`) to let Control D fully process the removals.
         - Build the global `existing_rules` set.
         - Sequentially process each folder (executor with `max_workers=1` to avoid rate-limit and ordering issues), calling `_process_single_folder()` for each.

main.py

@@ -444,7 +444,7 @@
                     ip_str = res[4][0]
                     ip = ipaddress.ip_address(ip_str)
                     if not ip.is_global or ip.is_multicast:
                         log.warning(
                             f"Skipping unsafe URL (domain {hostname} resolves to non-global/multicast IP {ip}): {sanitize_for_log(url)}"
                         )
                         return False
@@ -533,7 +533,7 @@
 
 def validate_folder_data(data: Dict[str, Any], url: str) -> bool:
     if not isinstance(data, dict):
         log.error(
             f"Invalid data from {sanitize_for_log(url)}: Root must be a JSON object."
         )
         return False
@@ -541,12 +541,12 @@
         log.error(f"Invalid data from {sanitize_for_log(url)}: Missing 'group' key.")
         return False
     if not isinstance(data["group"], dict):
         log.error(
             f"Invalid data from {sanitize_for_log(url)}: 'group' must be an object."
         )
         return False
     if "group" not in data["group"]:
         log.error(
             f"Invalid data from {sanitize_for_log(url)}: Missing 'group.group' (folder name)."
         )
         return False
@@ -676,13 +676,13 @@
         resp = client.get(url)
         resp.raise_for_status()
         return True
     except httpx.HTTPStatusError as e:
         code = e.response.status_code
         if code == 401:
             log.critical(
                 f"{Colors.FAIL}❌ Authentication Failed: The API Token is invalid.{Colors.ENDC}"
             )
             log.critical(
                 f"{Colors.FAIL}   Please check your token at: https://controld.com/account/manage-account{Colors.ENDC}"
             )
         elif code == 403:
@@ -693,13 +693,13 @@
             log.critical(
                 f"{Colors.FAIL}🔍 Profile Not Found: The ID '{profile_id}' does not exist.{Colors.ENDC}"
             )
             log.critical(
                 f"{Colors.FAIL}   Please verify the Profile ID from your Control D Dashboard URL.{Colors.ENDC}"
             )
         else:
             log.error(f"API Access Check Failed ({code}): {e}")
         return False
     except httpx.RequestError as e:
         log.error(f"Network Error during access check: {e}")
         return False
 
@@ -718,6 +718,116 @@
         return {}
 
 
+def verify_access_and_get_folders(
+    client: httpx.Client, profile_id: str
+) -> Optional[Dict[str, str]]:
+    """Combine access check and folder listing into a single API request.
+
+    Returns:
+        Dict of {folder_name: folder_id} on success.
+        None if access is denied or the request fails after retries.
+    """
+    url = f"{API_BASE}/{profile_id}/groups"
+
+    for attempt in range(MAX_RETRIES):
+        try:
+            resp = client.get(url)
+            resp.raise_for_status()
+
+            try:
+                data = resp.json()
+
+                # Ensure we got the expected top-level JSON structure.
+                # We defensively validate types here so that unexpected but valid
+                # JSON (e.g., a list or a scalar) doesn't cause AttributeError/TypeError
+                # and crash the sync logic.
+                if not isinstance(data, dict):
+                    log.error(
+                        "Failed to parse folders data: expected JSON object at top level, "
+                        f"got {type(data).__name__}"
+                    )
+                    return None
+
+                body = data.get("body")
+                if not isinstance(body, dict):
+                    log.error(
+                        "Failed to parse folders data: expected 'body' to be an object, "
+                        f"got {type(body).__name__ if body is not None else 'None'}"
+                    )
+                    return None
+
+                folders = body.get("groups", [])
+                if not isinstance(folders, list):
+                    log.error(
+                        "Failed to parse folders data: expected 'body[\"groups\"]' to be a list, "
+                        f"got {type(folders).__name__}"
+                    )
+                    return None
+
+                # Only process entries that are dicts and have the required keys.
+                result: Dict[str, str] = {}
+                for f in folders:
+                    if not isinstance(f, dict):
+                        # Skip non-dict entries instead of crashing; this protects
+                        # against partial data corruption or unexpected API changes.
+                        continue
+                    name = f.get("group")
+                    pk = f.get("PK")
+                    if not name or not pk:
+                        continue
+                    result[str(name).strip()] = str(pk)
+
+                return result
+            except (KeyError, ValueError, TypeError, AttributeError) as e:
+                # As a final safeguard, catch any remaining parsing/shape errors so
+                # that a malformed response cannot crash the caller.
+                log.error(f"Failed to parse folders data: {sanitize_for_log(e)}")
+                return None
+
+        except httpx.HTTPStatusError as e:
+            code = e.response.status_code
+            if code in (401, 403, 404):
+                if code == 401:
+                    log.critical(
+                        f"{Colors.FAIL}❌ Authentication Failed: The API Token is invalid.{Colors.ENDC}"
+                    )
+                    log.critical(
+                        f"{Colors.FAIL}   Please check your token at: https://controld.com/account/manage-account{Colors.ENDC}"
+                    )
+                elif code == 403:
+                    log.critical(
+                        f"{Colors.FAIL}🚫 Access Denied: Token lacks permission for Profile {sanitize_for_log(profile_id)}.{Colors.ENDC}"
+                    )
+                elif code == 404:
+                    log.critical(
+                        f"{Colors.FAIL}🔍 Profile Not Found: The ID '{sanitize_for_log(profile_id)}' does not exist.{Colors.ENDC}"
+                    )
+                    log.critical(
+                        f"{Colors.FAIL}   Please verify the Profile ID from your Control D Dashboard URL.{Colors.ENDC}"
+                    )
+                return None
+
+            if attempt == MAX_RETRIES - 1:
+                log.error(f"API Request Failed ({code}): {sanitize_for_log(e)}")
+                return None
+
+        except httpx.RequestError as e:
+            if attempt == MAX_RETRIES - 1:
+                log.error(
+                    f"Network error during access verification: {sanitize_for_log(e)}"
+                )
+                return None
+
+        wait_time = RETRY_DELAY * (2**attempt)
+        log.warning(
+            f"Request failed (attempt {attempt + 1}/{MAX_RETRIES}). Retrying in {wait_time}s..."
+        )
+        time.sleep(wait_time)
+
+    log.error("Access verification failed after all retries")
+    return None
+
+
 def get_all_existing_rules(
     client: httpx.Client,
     profile_id: str,
@@ -806,7 +916,7 @@
         return None
 
     completed = 0
     with concurrent.futures.ThreadPoolExecutor() as executor:
         futures = {
             executor.submit(_validate_and_fetch, url): url for url in urls_to_process
         }
@@ -853,7 +963,7 @@
 
 def create_folder(
     client: httpx.Client, profile_id: str, name: str, do: int, status: int
 ) -> Optional[str]:
     """
     Create a new folder and return its ID.
     Attempts to read ID from response first, then falls back to polling.
@@ -902,7 +1012,7 @@
                     if grp["group"].strip() == name.strip():
                         log.info(
                             "Created folder %s (ID %s) [Polled]",
                             sanitize_for_log(name),
                             grp["PK"],
                         )
                         return str(grp["PK"])
@@ -1048,7 +1158,7 @@
         else:
             log.info(
                 "Folder %s – finished (%d new rules added)",
                 sanitize_for_log(folder_name),
                 len(filtered_hostnames),
             )
         return True
@@ -1224,11 +1334,11 @@
         # Initial client for getting existing state AND processing folders
         # Optimization: Reuse the same client session to keep TCP connections alive
         with _api_client() as client:
-            # Check for API access problems first (401/403/404)
-            if not check_api_access(client, profile_id):
+            # Verify access and list existing folders in one request
+            existing_folders = verify_access_and_get_folders(client, profile_id)
+            if existing_folders is None:
                 return False
 
-            existing_folders = list_existing_folders(client, profile_id)
             if not no_delete:
                 deletion_occurred = False
 

test_main.py

@@ -248,16 +248,27 @@
     assert "https://controld.com/account/manage-account" in stdout
 
 
-# Case 7: check_api_access handles success and errors correctly
-def test_check_api_access_success(monkeypatch):
+# Case 7: verify_access_and_get_folders handles success and errors correctly
+def test_verify_access_and_get_folders_success(monkeypatch):
     m = reload_main_with_env(monkeypatch)
     mock_client = MagicMock()
-    mock_client.get.return_value.raise_for_status.return_value = None
-
-    assert m.check_api_access(mock_client, "valid_profile") is True
-
-
-def test_check_api_access_401(monkeypatch):
+    mock_response = MagicMock()
+    mock_response.json.return_value = {
+        "body": {
+            "groups": [
+                {"group": "Folder A", "PK": "id_a"},
+                {"group": "Folder B", "PK": "id_b"}
+            ]
+        }
+    }
+    mock_client.get.return_value = mock_response
+    mock_response.raise_for_status.return_value = None
+
+    result = m.verify_access_and_get_folders(mock_client, "valid_profile")
+    assert result == {"Folder A": "id_a", "Folder B": "id_b"}
+
+
+def test_verify_access_and_get_folders_401(monkeypatch):
     m = reload_main_with_env(monkeypatch)
     mock_client = MagicMock()
 
@@ -273,14 +284,14 @@
     mock_log = MagicMock()
     monkeypatch.setattr(m, "log", mock_log)
 
-    assert m.check_api_access(mock_client, "invalid_token") is False
+    assert m.verify_access_and_get_folders(mock_client, "invalid_token") is None
     assert mock_log.critical.call_count >= 1
     # Check for authentication failed message
     args = str(mock_log.critical.call_args_list)
     assert "Authentication Failed" in args
 
 
-def test_check_api_access_403(monkeypatch):
+def test_verify_access_and_get_folders_403(monkeypatch):
     m = reload_main_with_env(monkeypatch)
     mock_client = MagicMock()
 
@@ -295,12 +306,12 @@
     mock_log = MagicMock()
     monkeypatch.setattr(m, "log", mock_log)
 
-    assert m.check_api_access(mock_client, "forbidden_profile") is False
+    assert m.verify_access_and_get_folders(mock_client, "forbidden_profile") is None
     assert mock_log.critical.call_count == 1
     assert "Access Denied" in str(mock_log.critical.call_args)
 
 
-def test_check_api_access_404(monkeypatch):
+def test_verify_access_and_get_folders_404(monkeypatch):
     m = reload_main_with_env(monkeypatch)
     mock_client = MagicMock()
 
@@ -315,12 +326,12 @@
     mock_log = MagicMock()
     monkeypatch.setattr(m, "log", mock_log)
 
-    assert m.check_api_access(mock_client, "missing_profile") is False
+    assert m.verify_access_and_get_folders(mock_client, "missing_profile") is None
     assert mock_log.critical.call_count >= 1
     assert "Profile Not Found" in str(mock_log.critical.call_args_list)
 
 
-def test_check_api_access_generic_http_error(monkeypatch):
+def test_verify_access_and_get_folders_500_retry(monkeypatch):
     m = reload_main_with_env(monkeypatch)
     mock_client = MagicMock()
 
@@ -334,13 +345,17 @@
 
     mock_log = MagicMock()
     monkeypatch.setattr(m, "log", mock_log)
+    monkeypatch.setattr(m, "RETRY_DELAY", 0.001)
+    monkeypatch.setattr("time.sleep", lambda x: None)
+    monkeypatch.setattr(m, "MAX_RETRIES", 2)
 
-    assert m.check_api_access(mock_client, "profile") is False
+    assert m.verify_access_and_get_folders(mock_client, "profile") is None
+    assert mock_client.get.call_count == 2
     assert mock_log.error.called
     assert "500" in str(mock_log.error.call_args)
 
 
-def test_check_api_access_network_error(monkeypatch):
+def test_verify_access_and_get_folders_network_error(monkeypatch):
     m = reload_main_with_env(monkeypatch)
     mock_client = MagicMock()
 
@@ -350,10 +365,14 @@
 
     mock_log = MagicMock()
     monkeypatch.setattr(m, "log", mock_log)
+    monkeypatch.setattr(m, "RETRY_DELAY", 0.001)
+    monkeypatch.setattr("time.sleep", lambda x: None)
+    monkeypatch.setattr(m, "MAX_RETRIES", 2)
 
-    assert m.check_api_access(mock_client, "profile") is False
+    assert m.verify_access_and_get_folders(mock_client, "profile") is None
+    assert mock_client.get.call_count == 2
     assert mock_log.error.called
-    assert "Network failure" in str(mock_log.error.call_args)
+    assert "Network error" in str(mock_log.error.call_args) or "access verification" in str(mock_log.error.call_args)
 
 
 # Case 8: extract_profile_id correctly extracts ID from URL or returns input

tests/test_parallel_deletion.py

@@ -36,10 +36,7 @@
         __enter__=lambda self: mock_client, __exit__=lambda *args: None
     )
     monkeypatch.setattr(main, "_api_client", lambda: mock_client_ctx)
-    monkeypatch.setattr(main, "check_api_access", lambda *args: True)
-    monkeypatch.setattr(
-        main, "list_existing_folders", lambda *args: {"FolderA": "id1", "FolderB": "id2"}
-    )
+    monkeypatch.setattr(main, "verify_access_and_get_folders", lambda *args: {"FolderA": "id1", "FolderB": "id2"})
     monkeypatch.setattr(main, "delete_folder", lambda *args: True)
     monkeypatch.setattr(main, "get_all_existing_rules", lambda *args: set())
     monkeypatch.setattr(main, "countdown_timer", lambda *args: None)
@@ -93,10 +90,7 @@
         __enter__=lambda self: mock_client, __exit__=lambda *args: None
     )
     monkeypatch.setattr(main, "_api_client", lambda: mock_client_ctx)
-    monkeypatch.setattr(main, "check_api_access", lambda *args: True)
-    monkeypatch.setattr(
-        main, "list_existing_folders", lambda *args: {"Folder1": "id1"}
-    )
+    monkeypatch.setattr(main, "verify_access_and_get_folders", lambda *args: {"Folder1": "id1"})
 
     # Mock delete_folder to raise an exception
     def failing_delete(*args):
@@ -146,10 +140,7 @@
         __enter__=lambda self: mock_client, __exit__=lambda *args: None
     )
     monkeypatch.setattr(main, "_api_client", lambda: mock_client_ctx)
-    monkeypatch.setattr(main, "check_api_access", lambda *args: True)
-    monkeypatch.setattr(
-        main, "list_existing_folders", lambda *args: {"TestFolder": "id1"}
-    )
+    monkeypatch.setattr(main, "verify_access_and_get_folders", lambda *args: {"TestFolder": "id1"})
 
     # Mock delete_folder to raise exception with potentially dangerous content
     def failing_delete(*args):