Fix intermittent connection resets on scan comparison by polling the diff-scans endpoints

[lelia]

Note: blocked on socketdev 3.4.0 reaching PyPI (SocketDev/socket-sdk-python#98). CI checks will fail in the interim. The typed params below only exist in 3.4.0; do not merge until it publishes, then regenerate uv.lock.

Diff-mode scans on self-hosted CI runners intermittently fail on the final comparison step with Connection error after 261.68 seconds: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer')) followed by a blank API Error:, even though the full scan itself succeeded and the results are on the dashboard.

Root Cause

The scan comparison used fullscans.stream_diff, which sends one request and then holds the connection open — completely idle — while the API computes the diff. When the comparison takes longer than a network middlebox's TCP idle timeout (Azure NAT gateways default to 4 minutes; the reported resets fired at ~262s on runners egressing through Azure), the middlebox reaps the "zombie" connection and sends a RST. Our API logs show request aborted (client-side termination) with no response ever written. Intermittency tracks diff computation time: fast diffs finish before the idle timer, slow ones (no baseline / large dependency trees) don't.

Fix

Per dougbot's suggestion, the comparison now uses the diff-scans endpoints with polling instead of a held-open connection:

1. POST /orgs/{org}/diff-scans/from-ids creates a diff-scan resource for the two full scans (returns metadata immediately; on_duplicate=redirect makes reruns of the same pair benign).
2. The CLI polls GET /orgs/{org}/diff-scans/{id}?cached=true — the API answers 202 while the diff is computing and 200 with the artifacts once ready. Every request is short and bounded, so nothing is ever idle long enough to be reaped.

Details:

• Poll interval backs off 5s → 30s (1.5×), since each poll consumes 1 quota unit; 30-minute backstop timeout.
• Transient poll failures (connection reset/timeout/502-class, via APIFailure.is_transient_error()) retry within the loop — the diff keeps computing server-side regardless.
• Fallback: any failure of the new flow logs a warning and falls back to the legacy stream_diff path, so tokens missing the newly-required scopes (diff-scans:create, diff-scans:list, full-scans:list) keep working exactly as today. No flags, no behavior change otherwise — full-scan creation, head-scan management (including the new --base-scan-id/--base-commit-sha baseline overrides from 2.5.0), and reachability finalize are untouched.
• One caveat vs. the legacy path (flagged by Bugbot, fixed in f31cfa7): the API ignores omit_license_details on cached reads, so cached diff-scan results always embed license details — there is no lean-response option here, unlike stream_diff with include_license_details=false (the CE-224 mitigation). If that heavier payload ever gets truncated on a huge dependency tree, JSON parsing fails and the existing fallback kicks in, which still requests the lean streaming payload. Worth considering server-side: honoring omit_license_details for cached reads would restore the lean option on this endpoint.

Dependencies / rollout:

• Requires socketdev>=3.4.0 — companion SDK PR: Add cached diff-scan polling support to DiffScans.get socket-sdk-python#99 (adds query-param + 202 support to diffscans.get). The SDK must be released to PyPI first; uv.lock needs regenerating after that (lockfile intentionally not updated here since 3.4.0 isn't published yet).
• Version bumped to 2.6.0.

Testing: new tests/core/test_diff_scan_polling.py covers poll-until-ready, backoff schedule, transient-error retry, non-transient propagation, timeout backstop, duplicate-redirect shortcut, and the streaming fallback. Full suite after rebasing onto v2.5.8: 427 passed, 2 skipped (run against the local SDK build).

Public Changelog

Diff-mode scan comparison no longer holds an idle HTTP connection open while the API computes the diff. The CLI now polls the diff-scans endpoints with short bounded requests, fixing intermittent "Connection reset by peer" failures on the final comparison step behind network gear with TCP idle timeouts (e.g. Azure NAT gateways). The change is transparent; if the API token lacks the diff-scans scopes the CLI falls back to the previous behavior.

Refs CE-354

---

Note

^{Cursor Bugbot is generating a summary for commit 8a0e2c8. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Done

Or push these changes by commenting:

@cursor push 2dba87dda6

Preview (2dba87dda6)

diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py
--- a/socketsecurity/core/__init__.py
+++ b/socketsecurity/core/__init__.py
@@ -1337,6 +1337,11 @@ def get_diff_scan_artifacts(
         the backend computes, so the comparison survives network idle timeouts
         (CE-354). See the DIFF_SCAN_POLL_* constants for the polling policy.
 
+        When ``include_license_details`` is False (the default), a final fetch
+        without ``cached`` requests ``omit_license_details=true``. The API
+        ignores that flag on cached responses, so the lean payload has to come
+        from a separate non-cached get (CE-224).
+
         Requires an org token with the ``diff-scans:create``, ``diff-scans:list``
         and ``full-scans:list`` scopes; callers are expected to catch failures and
         fall back to the legacy streaming comparison.

@@ -1337,6 +1337,11 @@ def get_diff_scan_artifacts(
         the backend computes, so the comparison survives network idle timeouts
         (CE-354). See the DIFF_SCAN_POLL_* constants for the polling policy.
 
+        When ``include_license_details`` is False (the default), a final fetch
+        without ``cached`` requests ``omit_license_details=true``. The API
+        ignores that flag on cached responses, so the lean payload has to come
+        from a separate non-cached get (CE-224).
+
         Requires an org token with the ``diff-scans:create``, ``diff-scans:list``
         and ``full-scans:list`` scopes; callers are expected to catch failures and
         fall back to the legacy streaming comparison.
@@ -1368,10 +1373,16 @@ def get_diff_scan_artifacts(
         # which case the create response already carries the artifacts.
         artifacts_dict = diff_scan.get("artifacts")
 
-        poll_params = {
-            "cached": "true",
-            "omit_license_details": "false" if include_license_details else "true",
-        }
+        # Poll with cached=true for short bounded 202/200 responses (CE-354).
+        # The API ignores omit_license_details whenever cached=true — cached
+        # payloads always embed full license data — so readiness polling never
+        # requests it. When license details should be omitted (the default;
+        # CE-224), the lean payload is fetched separately below without cached.
+        poll_params = {"cached": "true"}
+        if not include_license_details:
+            # Keep the readiness response small so the ignored omit doesn't
+            # reintroduce large-response truncation while we wait for 200.
+            poll_params["omit_unchanged"] = "true"
         deadline = time.monotonic() + DIFF_SCAN_POLL_TIMEOUT_SECONDS
         interval = DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS
         while artifacts_dict is None:

@@ -1368,10 +1373,16 @@ def get_diff_scan_artifacts(
         # which case the create response already carries the artifacts.
         artifacts_dict = diff_scan.get("artifacts")
 
-        poll_params = {
-            "cached": "true",
-            "omit_license_details": "false" if include_license_details else "true",
-        }
+        # Poll with cached=true for short bounded 202/200 responses (CE-354).
+        # The API ignores omit_license_details whenever cached=true — cached
+        # payloads always embed full license data — so readiness polling never
+        # requests it. When license details should be omitted (the default;
+        # CE-224), the lean payload is fetched separately below without cached.
+        poll_params = {"cached": "true"}
+        if not include_license_details:
+            # Keep the readiness response small so the ignored omit doesn't
+            # reintroduce large-response truncation while we wait for 200.
+            poll_params["omit_unchanged"] = "true"
         deadline = time.monotonic() + DIFF_SCAN_POLL_TIMEOUT_SECONDS
         interval = DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS
         while artifacts_dict is None:
@@ -1404,6 +1415,21 @@ def get_diff_scan_artifacts(
             time.sleep(interval)
             interval = min(interval * DIFF_SCAN_POLL_BACKOFF_MULTIPLIER, DIFF_SCAN_POLL_MAX_INTERVAL_SECONDS)
 
+        # Cached results always include license details. Re-fetch once without
+        # cached so omit_license_details is honored and the diff stays lean.
+        if not include_license_details:
+            response = self.sdk.diffscans.get(
+                self.config.org_slug,
+                diff_scan_id,
+                params={"omit_license_details": "true"},
+            )
+            scan = response.get("diff_scan") or {}
+            if scan.get("artifacts") is None:
+                raise Exception(
+                    f"Error fetching diff scan {diff_scan_id}: unexpected response: {str(response)[:500]}"
+                )
+            artifacts_dict = scan["artifacts"]
+
         return DiffArtifacts.from_dict({
             key: artifacts_dict.get(key) or []
             for key in ("added", "removed", "unchanged", "replaced", "updated")

@@ -1404,6 +1415,21 @@ def get_diff_scan_artifacts(
             time.sleep(interval)
             interval = min(interval * DIFF_SCAN_POLL_BACKOFF_MULTIPLIER, DIFF_SCAN_POLL_MAX_INTERVAL_SECONDS)
 
+        # Cached results always include license details. Re-fetch once without
+        # cached so omit_license_details is honored and the diff stays lean.
+        if not include_license_details:
+            response = self.sdk.diffscans.get(
+                self.config.org_slug,
+                diff_scan_id,
+                params={"omit_license_details": "true"},
+            )
+            scan = response.get("diff_scan") or {}
+            if scan.get("artifacts") is None:
+                raise Exception(
+                    f"Error fetching diff scan {diff_scan_id}: unexpected response: {str(response)[:500]}"
+                )
+            artifacts_dict = scan["artifacts"]
+
         return DiffArtifacts.from_dict({
             key: artifacts_dict.get(key) or []
             for key in ("added", "removed", "unchanged", "replaced", "updated")

diff --git a/tests/core/test_diff_scan_polling.py b/tests/core/test_diff_scan_polling.py
--- a/tests/core/test_diff_scan_polling.py
+++ b/tests/core/test_diff_scan_polling.py
@@ -26,11 +26,14 @@ def no_sleep(mocker):
 def test_polls_until_diff_scan_ready(core, diff_scan_get_response, no_sleep):
     """202 processing responses are polled through until the 200 result arrives."""
     processing = {"status": "processing", "id": "diff-scan-123"}
-    core.sdk.diffscans.get.side_effect = [processing, processing, diff_scan_get_response]
+    # Final cached poll + lean omit_license_details re-fetch.
+    core.sdk.diffscans.get.side_effect = [
+        processing, processing, diff_scan_get_response, diff_scan_get_response
+    ]
 
     artifacts = core.get_diff_scan_artifacts("head", "new")
 
-    assert core.sdk.diffscans.get.call_count == 3
+    assert core.sdk.diffscans.get.call_count == 4
     assert no_sleep.call_count == 2  # slept between polls, never during them
     assert len(artifacts.added) > 0
 

@@ -26,11 +26,14 @@ def no_sleep(mocker):
 def test_polls_until_diff_scan_ready(core, diff_scan_get_response, no_sleep):
     """202 processing responses are polled through until the 200 result arrives."""
     processing = {"status": "processing", "id": "diff-scan-123"}
-    core.sdk.diffscans.get.side_effect = [processing, processing, diff_scan_get_response]
+    # Final cached poll + lean omit_license_details re-fetch.
+    core.sdk.diffscans.get.side_effect = [
+        processing, processing, diff_scan_get_response, diff_scan_get_response
+    ]
 
     artifacts = core.get_diff_scan_artifacts("head", "new")
 
-    assert core.sdk.diffscans.get.call_count == 3
+    assert core.sdk.diffscans.get.call_count == 4
     assert no_sleep.call_count == 2  # slept between polls, never during them
     assert len(artifacts.added) > 0
 
@@ -40,7 +43,9 @@ def test_poll_interval_backs_off(core, diff_scan_get_response, no_sleep, monkeyp
     monkeypatch.setattr(core_module, "DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS", 4.0)
     monkeypatch.setattr(core_module, "DIFF_SCAN_POLL_MAX_INTERVAL_SECONDS", 10.0)
     processing = {"status": "processing", "id": "diff-scan-123"}
-    core.sdk.diffscans.get.side_effect = [processing] * 4 + [diff_scan_get_response]
+    core.sdk.diffscans.get.side_effect = (
+        [processing] * 4 + [diff_scan_get_response, diff_scan_get_response]
+    )
 
     core.get_diff_scan_artifacts("head", "new")
 

@@ -40,7 +43,9 @@ def test_poll_interval_backs_off(core, diff_scan_get_response, no_sleep, monkeyp
     monkeypatch.setattr(core_module, "DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS", 4.0)
     monkeypatch.setattr(core_module, "DIFF_SCAN_POLL_MAX_INTERVAL_SECONDS", 10.0)
     processing = {"status": "processing", "id": "diff-scan-123"}
-    core.sdk.diffscans.get.side_effect = [processing] * 4 + [diff_scan_get_response]
+    core.sdk.diffscans.get.side_effect = (
+        [processing] * 4 + [diff_scan_get_response, diff_scan_get_response]
+    )
 
     core.get_diff_scan_artifacts("head", "new")
 
@@ -50,11 +55,13 @@ def test_poll_interval_backs_off(core, diff_scan_get_response, no_sleep, monkeyp
 
 def test_transient_poll_error_is_retried(core, diff_scan_get_response, no_sleep):
     """A dropped poll doesn't abandon the flow - the diff keeps computing server-side."""
-    core.sdk.diffscans.get.side_effect = [APIConnectionError("reset"), diff_scan_get_response]
+    core.sdk.diffscans.get.side_effect = [
+        APIConnectionError("reset"), diff_scan_get_response, diff_scan_get_response
+    ]
 
     artifacts = core.get_diff_scan_artifacts("head", "new")
 
-    assert core.sdk.diffscans.get.call_count == 2
+    assert core.sdk.diffscans.get.call_count == 3
     assert len(artifacts.added) > 0
 
 

@@ -50,11 +55,13 @@ def test_poll_interval_backs_off(core, diff_scan_get_response, no_sleep, monkeyp
 
 def test_transient_poll_error_is_retried(core, diff_scan_get_response, no_sleep):
     """A dropped poll doesn't abandon the flow - the diff keeps computing server-side."""
-    core.sdk.diffscans.get.side_effect = [APIConnectionError("reset"), diff_scan_get_response]
+    core.sdk.diffscans.get.side_effect = [
+        APIConnectionError("reset"), diff_scan_get_response, diff_scan_get_response
+    ]
 
     artifacts = core.get_diff_scan_artifacts("head", "new")
 
-    assert core.sdk.diffscans.get.call_count == 2
+    assert core.sdk.diffscans.get.call_count == 3
     assert len(artifacts.added) > 0
 
 
@@ -76,15 +83,45 @@ def test_poll_timeout_raises(core, no_sleep, monkeypatch):
 
 
 def test_duplicate_redirect_uses_embedded_artifacts(core, diff_scan_get_response):
-    """An on_duplicate redirect can return the computed diff scan straight away."""
+    """An on_duplicate redirect skips readiness polling; lean re-fetch still runs."""
     core.sdk.diffscans.create_from_ids.return_value = diff_scan_get_response
 
     artifacts = core.get_diff_scan_artifacts("head", "new")
 
-    core.sdk.diffscans.get.assert_not_called()
+    # Create already carried artifacts, so cached readiness polling is skipped,
+    # but omit_license_details still needs a non-cached get (cached ignores it).
+    core.sdk.diffscans.get.assert_called_once_with(
+        core.config.org_slug,
+        "diff-scan-123",
+        params={"omit_license_details": "true"},
+    )
     assert len(artifacts.added) > 0
 
 
+def test_lean_refetch_omits_license_details_without_cached(
+    core, diff_scan_get_response, no_sleep
+):
+    """omit_license_details is fetched without cached=true (API ignores it otherwise)."""
+    processing = {"status": "processing", "id": "diff-scan-123"}
+    core.sdk.diffscans.get.side_effect = [
+        processing, diff_scan_get_response, diff_scan_get_response
+    ]
+
+    core.get_diff_scan_artifacts("head", "new")
+
+    assert core.sdk.diffscans.get.call_args_list[0].kwargs["params"] == {
+        "cached": "true",
+        "omit_unchanged": "true",
+    }
+    assert core.sdk.diffscans.get.call_args_list[1].kwargs["params"] == {
+        "cached": "true",
+        "omit_unchanged": "true",
+    }
+    assert core.sdk.diffscans.get.call_args_list[2].kwargs["params"] == {
+        "omit_license_details": "true",
+    }
+
+
 def test_fallback_to_streaming_diff_on_failure(core):
     """If the diff-scans flow fails (e.g. token missing the diff-scans scopes),
     the comparison falls back to the legacy streaming diff transparently."""

@@ -76,15 +83,45 @@ def test_poll_timeout_raises(core, no_sleep, monkeypatch):
 
 
 def test_duplicate_redirect_uses_embedded_artifacts(core, diff_scan_get_response):
-    """An on_duplicate redirect can return the computed diff scan straight away."""
+    """An on_duplicate redirect skips readiness polling; lean re-fetch still runs."""
     core.sdk.diffscans.create_from_ids.return_value = diff_scan_get_response
 
     artifacts = core.get_diff_scan_artifacts("head", "new")
 
-    core.sdk.diffscans.get.assert_not_called()
+    # Create already carried artifacts, so cached readiness polling is skipped,
+    # but omit_license_details still needs a non-cached get (cached ignores it).
+    core.sdk.diffscans.get.assert_called_once_with(
+        core.config.org_slug,
+        "diff-scan-123",
+        params={"omit_license_details": "true"},
+    )
     assert len(artifacts.added) > 0
 
 
+def test_lean_refetch_omits_license_details_without_cached(
+    core, diff_scan_get_response, no_sleep
+):
+    """omit_license_details is fetched without cached=true (API ignores it otherwise)."""
+    processing = {"status": "processing", "id": "diff-scan-123"}
+    core.sdk.diffscans.get.side_effect = [
+        processing, diff_scan_get_response, diff_scan_get_response
+    ]
+
+    core.get_diff_scan_artifacts("head", "new")
+
+    assert core.sdk.diffscans.get.call_args_list[0].kwargs["params"] == {
+        "cached": "true",
+        "omit_unchanged": "true",
+    }
+    assert core.sdk.diffscans.get.call_args_list[1].kwargs["params"] == {
+        "cached": "true",
+        "omit_unchanged": "true",
+    }
+    assert core.sdk.diffscans.get.call_args_list[2].kwargs["params"] == {
+        "omit_license_details": "true",
+    }
+
+
 def test_fallback_to_streaming_diff_on_failure(core):
     """If the diff-scans flow fails (e.g. token missing the diff-scans scopes),
     the comparison falls back to the legacy streaming diff transparently."""

diff --git a/tests/core/test_sdk_methods.py b/tests/core/test_sdk_methods.py
--- a/tests/core/test_sdk_methods.py
+++ b/tests/core/test_sdk_methods.py
@@ -239,12 +239,15 @@ def test_get_added_and_removed_packages(core):
     # include_license_details defaults to False: the diff path never consumes
     # embedded license data (license artifacts come from the PURL endpoint), so
     # requesting it only bloats the response and risks the truncation
-    # crash on large repos.
-    core.sdk.diffscans.get.assert_called_once_with(
-        core.config.org_slug,
-        "diff-scan-123",
-        params={"cached": "true", "omit_license_details": "true"},
-    )
+    # crash on large repos. cached=true ignores omit_license_details, so the
+    # poll checks readiness (optionally omitting unchanged to stay small) and
+    # a separate non-cached get fetches the lean payload.
+    get_calls = core.sdk.diffscans.get.call_args_list
+    assert len(get_calls) == 2
+    assert get_calls[0].args == (core.config.org_slug, "diff-scan-123")
+    assert get_calls[0].kwargs["params"] == {"cached": "true", "omit_unchanged": "true"}
+    assert get_calls[1].args == (core.config.org_slug, "diff-scan-123")
+    assert get_calls[1].kwargs["params"] == {"omit_license_details": "true"}
     core.sdk.fullscans.stream_diff.assert_not_called()
     
     # Verify the results

@@ -239,12 +239,15 @@ def test_get_added_and_removed_packages(core):
     # include_license_details defaults to False: the diff path never consumes
     # embedded license data (license artifacts come from the PURL endpoint), so
     # requesting it only bloats the response and risks the truncation
-    # crash on large repos.
-    core.sdk.diffscans.get.assert_called_once_with(
-        core.config.org_slug,
-        "diff-scan-123",
-        params={"cached": "true", "omit_license_details": "true"},
-    )
+    # crash on large repos. cached=true ignores omit_license_details, so the
+    # poll checks readiness (optionally omitting unchanged to stay small) and
+    # a separate non-cached get fetches the lean payload.
+    get_calls = core.sdk.diffscans.get.call_args_list
+    assert len(get_calls) == 2
+    assert get_calls[0].args == (core.config.org_slug, "diff-scan-123")
+    assert get_calls[0].kwargs["params"] == {"cached": "true", "omit_unchanged": "true"}
+    assert get_calls[1].args == (core.config.org_slug, "diff-scan-123")
+    assert get_calls[1].kwargs["params"] == {"omit_license_details": "true"}
     core.sdk.fullscans.stream_diff.assert_not_called()
     
     # Verify the results
@@ -263,10 +266,12 @@ def test_get_added_and_removed_packages_license_override(core):
     """The include_license_details override seam still works when explicitly requested."""
     core.get_added_and_removed_packages("head", "new", include_license_details=True)
 
+    # When license details are wanted, the cached poll response is used directly
+    # — no lean re-fetch, and omit_license_details is not sent.
     core.sdk.diffscans.get.assert_called_once_with(
         core.config.org_slug,
         "diff-scan-123",
-        params={"cached": "true", "omit_license_details": "false"},
+        params={"cached": "true"},
     )
 
 def test_empty_alerts_preserved(core):

@@ -263,10 +266,12 @@ def test_get_added_and_removed_packages_license_override(core):
     """The include_license_details override seam still works when explicitly requested."""
     core.get_added_and_removed_packages("head", "new", include_license_details=True)
 
+    # When license details are wanted, the cached poll response is used directly
+    # — no lean re-fetch, and omit_license_details is not sent.
     core.sdk.diffscans.get.assert_called_once_with(
         core.config.org_slug,
         "diff-scan-123",
-        params={"cached": "true", "omit_license_details": "false"},
+        params={"cached": "true"},
     )
 
 def test_empty_alerts_preserved(core):

_{You can send follow-ups to the cloud agent here.}

[lelia]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 58a2fb4. Configure here.}