Apply code-review improvements + bump to 2.2.92

- Cache comment bodies in get_comments_for_pr so has_thumbsup_reaction
  resolves from memory instead of issuing one extra GET per ignore
  comment on every run. _mark_comment_processed refreshes the cache so
  in-run repeat checks also hit it.
- Guard against {"values": null} responses from Bitbucket Cloud — the
  previous .get("values", []) default only fires on missing key, so a
  null value would TypeError on iteration.
- Fail fast in BitbucketConfig.from_env when workspace or repo_slug
  can't be determined, rather than building 404-bound URLs deeper in
  the request flow.
- Drop typing.Tuple in favor of built-in tuple (project targets 3.11+).
- Document BITBUCKET_DEFAULT_BRANCH in the example pipeline since
  Bitbucket Pipelines doesn't export the repo default branch.
- Add tests for cache hits, cache fallback, mark-processed cache
  refresh, null-values pagination, and the new workspace/repo
  validation exit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: obarrera

pyproject.toml

@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "socketsecurity"
-version = "2.2.91"
+version = "2.2.92"
 requires-python = ">= 3.11"
 license = {"file" = "LICENSE"}
 dependencies = [

socketsecurity/__init__.py

@@ -1,3 +1,3 @@
 __author__ = 'socket.dev'
-__version__ = '2.2.91'
+__version__ = '2.2.92'
 USER_AGENT = f'SocketPythonCLI/{__version__}'

socketsecurity/core/scm/bitbucket.py

@@ -3,7 +3,7 @@
 import os
 import sys
 from dataclasses import dataclass
-from typing import Optional, Tuple
+from typing import Optional
 from urllib.parse import urlparse
 
 from socketsecurity import USER_AGENT
@@ -59,6 +59,14 @@ def from_env(cls, pr_number: Optional[str] = None) -> "BitbucketConfig":
             workspace = workspace or full_workspace
             repo_slug = repo_slug or full_slug
 
+        if not workspace or not repo_slug:
+            log.error(
+                "Unable to determine Bitbucket workspace/repo. Set "
+                "BITBUCKET_REPO_FULL_NAME, or BITBUCKET_WORKSPACE + "
+                "BITBUCKET_REPO_SLUG."
+            )
+            sys.exit(2)
+
         pr_id = pr_number or os.getenv("BITBUCKET_PR_ID")
         if pr_id == "0":
             pr_id = None
@@ -118,9 +126,13 @@ class Bitbucket:
     def __init__(self, client: CliClient, config: Optional[BitbucketConfig] = None):
         self.config = config or BitbucketConfig.from_env()
         self.client = client
+        # Populated by get_comments_for_pr; consulted by has_thumbsup_reaction
+        # to avoid one extra GET per ignore comment when the body is already
+        # in memory.
+        self._comment_body_cache: dict = {}
 
     @staticmethod
-    def _split_absolute_url(url: str) -> Tuple[str, str]:
+    def _split_absolute_url(url: str) -> tuple[str, str]:
         """Split an absolute URL into (origin, path+query) for CliClient.request.
 
         CliClient builds URLs as f"{base_url}/{path}", so an empty base_url
@@ -211,13 +223,14 @@ def get_comments_for_pr(self) -> dict:
                 log.error(data)
                 break
 
-            for raw in data.get("values", []):
+            for raw in data.get("values") or []:
                 normalized = self._normalize_comment(raw)
                 if normalized is None:
                     continue
                 comment = Comment(**normalized)
                 comment.body_list = comment.body.split("\n")
                 comments[comment.id] = comment
+                self._comment_body_cache[comment.id] = comment.body
 
             next_url = data.get("next")
             if not next_url:
@@ -296,7 +309,16 @@ def handle_ignore_reactions(self, comments: dict) -> None:
                 self._mark_comment_processed(comment)
 
     def has_thumbsup_reaction(self, comment_id) -> bool:
-        """Bitbucket has no reactions; detect our hidden processed marker."""
+        """Bitbucket has no reactions; detect our hidden processed marker.
+
+        Prefers the in-memory body cache populated by get_comments_for_pr;
+        only falls back to a GET when called for an id we haven't loaded
+        (defensive — currently no call path does this).
+        """
+        cached_body = self._comment_body_cache.get(comment_id)
+        if cached_body is not None:
+            return self.PROCESSED_MARKER in cached_body
+
         if not self.config.pr_id:
             return False
         try:
@@ -318,6 +340,8 @@ def _mark_comment_processed(self, comment) -> None:
         new_body = f"{comment.body}\n\n{self.PROCESSED_MARKER}"
         try:
             self.update_comment(new_body, str(comment.id))
+            comment.body = new_body
+            self._comment_body_cache[comment.id] = new_body
         except Exception as error:
             log.debug(f"Failed to mark Bitbucket ignore comment {comment.id} as processed: {error}")
 

tests/unit/test_bitbucket.py

@@ -70,6 +70,13 @@ def test_from_env_missing_credentials_exits(self):
         with pytest.raises(SystemExit):
             BitbucketConfig.from_env()
 
+    @patch.dict(os.environ, {"BITBUCKET_TOKEN": "t"}, clear=True)
+    def test_from_env_missing_workspace_repo_exits(self):
+        """Credentials present but no workspace/repo info — fail fast rather
+        than building 404-bound URLs deeper in the request flow."""
+        with pytest.raises(SystemExit):
+            BitbucketConfig.from_env()
+
     @patch.dict(os.environ, {
         "BITBUCKET_TOKEN": "t",
         "BITBUCKET_REPO_FULL_NAME": "acme/widgets",
@@ -389,5 +396,66 @@ def test_get_comments_runs_check_for_socket_comments(self):
         )
 
 
+class TestCommentBodyCache:
+    def test_has_thumbsup_uses_cache_after_fetch(self):
+        """get_comments_for_pr should populate the body cache so subsequent
+        has_thumbsup_reaction calls don't issue extra GETs per ignore comment."""
+        scm = Bitbucket(client=MagicMock(), config=_make_config())
+        page = {
+            "values": [
+                {
+                    "id": 1,
+                    "content": {"raw": f"@SocketSecurity ignore npm/foo@1.0.0\n\n{Bitbucket.PROCESSED_MARKER}"},
+                    "user": {"nickname": "alice", "uuid": "{u-1}"},
+                },
+                {
+                    "id": 2,
+                    "content": {"raw": "@SocketSecurity ignore npm/bar@2.0.0"},
+                    "user": {"nickname": "bob", "uuid": "{u-2}"},
+                },
+            ]
+        }
+        scm.client.request.return_value = _json_response(page)
+        scm.get_comments_for_pr()
+        calls_after_fetch = scm.client.request.call_count
+
+        # Both should resolve from cache — no additional API calls.
+        assert scm.has_thumbsup_reaction(1) is True
+        assert scm.has_thumbsup_reaction(2) is False
+        assert scm.client.request.call_count == calls_after_fetch
+
+    def test_has_thumbsup_falls_back_to_api_when_not_cached(self):
+        scm = Bitbucket(client=MagicMock(), config=_make_config())
+        scm.client.request.return_value = _json_response(
+            {"content": {"raw": f"body\n\n{Bitbucket.PROCESSED_MARKER}"}}
+        )
+        # ID not seen by get_comments_for_pr — falls back to GET.
+        assert scm.has_thumbsup_reaction(999) is True
+        scm.client.request.assert_called_once()
+
+    def test_mark_comment_processed_updates_cache(self):
+        """After marking, the cache reflects the new body so a subsequent
+        has_thumbsup_reaction in the same run sees it without re-fetching."""
+        scm = Bitbucket(client=MagicMock(), config=_make_config())
+        comment = MagicMock(id=42, body="original")
+        scm._mark_comment_processed(comment)
+        assert scm._comment_body_cache[42].endswith(Bitbucket.PROCESSED_MARKER)
+        # And subsequent reaction check is a cache hit.
+        scm.client.request.reset_mock()
+        assert scm.has_thumbsup_reaction(42) is True
+        scm.client.request.assert_not_called()
+
+
+class TestNullSafePaginationValues:
+    def test_handles_null_values_field(self):
+        """If Bitbucket returns {"values": null} instead of omitting the key,
+        the for-loop must not blow up."""
+        scm = Bitbucket(client=MagicMock(), config=_make_config())
+        scm.client.request.return_value = _json_response({"values": None})
+        # Should not raise.
+        result = scm.get_comments_for_pr()
+        assert result == {}
+
+
 if __name__ == "__main__":
     pytest.main([__file__])

uv.lock

@@ -19,11 +19,16 @@ definitions:
               --integration bitbucket \
               --pr-number ${BITBUCKET_PR_ID:-0}
         # Repository variables needed (set in Bitbucket repo settings):
-        # SOCKET_SECURITY_API_KEY: Your Socket Security API token
-        # BITBUCKET_TOKEN:         A Repository or Workspace access token with
-        #                          "Pull requests: Write" scope
-        #                          (or set BITBUCKET_USERNAME and
-        #                           BITBUCKET_APP_PASSWORD for an app password)
+        # SOCKET_SECURITY_API_KEY:   Your Socket Security API token
+        # BITBUCKET_TOKEN:           A Repository or Workspace access token with
+        #                            "Pull requests: Write" scope
+        #                            (or set BITBUCKET_USERNAME and
+        #                             BITBUCKET_APP_PASSWORD for an app password)
+        # BITBUCKET_DEFAULT_BRANCH:  Optional. Bitbucket Pipelines does not
+        #                            export the repo's default branch, so set
+        #                            this (e.g. "main") for accurate default-
+        #                            branch detection. You can also pass
+        #                            --default-branch on the CLI directly.
         # Without either credential set, --scm bitbucket exits 2.
 
 pipelines: