Skip to content

Add cached diff-scan polling support to DiffScans.get - #99

Merged
lelia merged 5 commits into
mainfrom
lelia/diff-scan-polling
Aug 5, 2026
Merged

Add cached diff-scan polling support to DiffScans.get#99
lelia merged 5 commits into
mainfrom
lelia/diff-scan-polling

Conversation

@lelia

@lelia lelia commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

DiffScans.get now accepts optional query params — notably cached=true — and surfaces the API's 202 Accepted processing status as {"status": "processing", "id": ...} instead of treating it as an error. For every 202 response, that polling sentinel and the requested diff-scan ID remain authoritative even if the response body contains conflicting values; other response metadata is preserved. This lets clients poll GET /orgs/{org_slug}/diff-scans/{diff_scan_id}?cached=true with short bounded requests until the computed diff is ready (HTTP 200). Also fixes list-valued query params (e.g. committers) in create_from_repo/create_from_ids to encode as repeated params (urlencode(doseq=True)).

Why?

The Python CLI's scan comparison currently uses fullscans.stream_diff, which holds one HTTP connection open — fully idle — while the backend computes the diff. Network middleboxes with TCP idle timeouts (notably Azure NAT gateways, 4-minute default) kill that connection with a RST, surfacing as intermittent ConnectionResetError(104, 'Connection reset by peer') failures on the final comparison step for scans run on self-hosted CI runners.

The companion CLI PR switches the comparison to diffscans.create_from_ids + polling diffscans.get(..., params={"cached": "true"}), which needs this SDK support. omit_license_details/omit_unchanged passthrough lets the CLI keep the lean-response behavior from CE-224.

New unit tests cover the query-string passthrough, the 202 processing status (with an empty, normal, or conflicting response body), and repeated-param encoding. python -m pytest tests/unit: 136 passed, 1 skipped. Package version staged at 3.4.1.

Public Changelog

diffscans.get now supports query parameters (cached, omit_unchanged, omit_license_details) and returns a stable {"status": "processing", "id": ...} result for HTTP 202, enabling clients to poll for diff-scan results instead of holding a long-lived connection open. The polling sentinel and requested ID remain authoritative if a 202 body contains conflicting values, while other response metadata is preserved. List-valued query params such as committers are now encoded correctly in diffscans.create_from_repo/create_from_ids.

Refs CE-354

DiffScans.get now accepts optional query params (cached, omit_unchanged,
omit_license_details) and returns a {"status": "processing", "id": ...}
dict on HTTP 202 instead of logging an error, so clients can poll
GET /orgs/{org}/diff-scans/{id}?cached=true until the computed diff is
ready rather than holding a single idle connection open while the
backend computes (which idle-timeout middleboxes like Azure NAT
gateways kill after ~4 minutes).

Also encode list-valued query params (e.g. committers) as repeated
params in create_from_repo/create_from_ids via urlencode(doseq=True).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

🚀 Preview package published!

Install with:

pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketdev==3.4.0.dev5

@lelia
lelia marked this pull request as ready for review August 5, 2026 04:11
@lelia
lelia requested a review from a team as a code owner August 5, 2026 04:11
@lelia
lelia temporarily deployed to socket-firewall August 5, 2026 04:11 — with GitHub Actions Inactive
@lelia
lelia temporarily deployed to socket-firewall August 5, 2026 19:14 — with GitHub Actions Inactive

@flowstate Eric Hibbs (flowstate) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ai@cursor: Approving — the approach is right, the 202-as-status handling is the correct shape, and the tests cover it. One thing worth hardening before merge, because I think it's live rather than theoretical.

The 202 handler can overwrite the sentinel it documents

result = {"status": "processing", "id": diff_scan_id}
body = response.json()
if isinstance(body, dict):
    result.update(body)

The merge is unconditional, so any status field in the 202 body wins over "processing":

202 body={'status': 'pending'}  ->  {'status': 'pending', 'id': 'abc'}
202 body={'id': 'abc'}          ->  {'status': 'processing', 'id': 'abc'}
202 body=None                   ->  {'status': 'processing', 'id': 'abc'}

That matters because the companion poll loop in SocketDev/socket-python-cli#284 branches on exactly that field:

if response.get("status") != "processing":
    scan = response.get("diff_scan") or {}
    if scan.get("artifacts") is None:
        raise Exception(f"Error fetching diff scan {diff_scan_id}: unexpected response: ...")

If a 202 body ever carries a status of its own, the loop exits early, finds no diff_scan in a still-computing response, and raises unexpected response — a hard failure on the final comparison step, which is the symptom this change set exists to remove.

I don't know what the API actually puts in a 202 body, and if it's empty this never fires. But the SDK merges an unknown body defensively while the loop treats anything that isn't "processing" as a finished diff, so the two halves disagree about who owns that field. Cheap to make unconditionally safe by letting the sentinel win:

result = {**body, "status": "processing", "id": diff_scan_id}

The doseq=True fix is correct — verified that list values encode as committers=a&committers=b.

@lelia
lelia temporarily deployed to socket-firewall August 5, 2026 20:10 — with GitHub Actions Inactive
@lelia

lelia commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Eric Hibbs (@flowstate) Good catch. Addressed in e26325a while keeping the hardening in this PR: the SDK now preserves additional 202 response fields but reapplies the canonical {"status": "processing", "id": diff_scan_id} values afterward, so a conflicting body cannot break the companion CLI poll loop.

I added a regression test with conflicting status/id values and bundled the planned 3.4.1 package/lockfile bump. Local validation is green: 136 passed, 1 skipped; build, Twine validation, and wheel smoke test also pass.

@flowstate Eric Hibbs (flowstate) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ai@cursor: Verified against e26325a and approving.

Re-ran the repro: the sentinel now wins in every conflicting case, and fields the body adds on top still come through.

202 body={'status': 'pending'}                        -> {'status': 'processing', 'id': 'abc'}
202 body={'status': 'pending', 'id': 'some-other-id'} -> {'status': 'processing', 'id': 'abc'}
202 body={'queuePosition': 3, 'startedAt': 't0',
          'status': 'pending', 'id': 'other'}         -> {'queuePosition': 3, 'startedAt': 't0',
                                                          'status': 'processing', 'id': 'abc'}

test_diffscans_get_processing_sentinel_wins_unit pins both halves of that — a body contradicting status and id, with an extra retry_after surviving — which is precisely the shape that would have dropped the companion CLI poll loop out early. Suite green at 136 passed / 1 skipped.

@lelia
lelia merged commit 2ae50f4 into main Aug 5, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants