Skip to content

feat(scan): use cached diff-scan endpoint with 202 polling (SURF-743) - #1499

Merged
John-David Dalton (jdalton) merged 2 commits into
mainfrom
jdalton/surf-743-cached-diff-scan
Aug 18, 2026
Merged

feat(scan): use cached diff-scan endpoint with 202 polling (SURF-743)#1499
John-David Dalton (jdalton) merged 2 commits into
mainfrom
jdalton/surf-743-cached-diff-scan

Conversation

@jdalton

@jdalton John-David Dalton (jdalton) commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:glm-fast-latest[1m]

This PR changes how socket scan diff asks the Socket API for a diff, so the command stops timing out on large scans. Instead of one long-lived request that blocks until the diff is fully computed, the CLI now creates a diff-scan resource and then polls a cached endpoint until the result is ready. This is the client-side half of the fix for the 1200-second timeout family (SURF-1487 / SURF-1044 / SURF-1364); the server-side immutable-queue routing already landed in the API service (PR #21167).

Why we're making this change

When you run socket scan diff, the CLI used to send a single request to the old GET /full-scans/diff endpoint and hold the connection open while the server computed the whole diff. For big scans that take longer than 1200 seconds, the SDK's fixed per-request timeout fires before the server finishes, and the command fails with API request failed. The --timeout flag does not help because it controls the overall scan budget, not the per-request ceiling. This is the same root cause behind the ID.me, ZeroFox, and Prometheus CI scan timeouts.

The new endpoints split the work into two steps. The server enqueues a background job to compute the diff and stores the result in an immutable cache. The CLI polls a short request that returns 202 Accepted while the job is running, then 200 OK with the finished diff. No single request is held open for the whole compute, so the 1200-second timeout no longer fires.

Old flow vs new flow

Old flow (one long-lived request):

CLI ── GET /full-scans/diff?before=X&after=Y ──> API
CLI <────────────── diff result ──────────────── API
(one connection held open the whole time; >1200s = timeout failure)

New flow (create once, poll until ready):

CLI ── POST /diff-scans/from-ids {before,after} ──> API   → 201 { diff_scan: { id } }
CLI ── GET /diff-scans/{id}?cached=true ──────────> API   → 202 { status: "processing" }
CLI ── GET /diff-scans/{id}?cached=true ──────────> API   → 202 (still computing)
CLI ── GET /diff-scans/{id}?cached=true ──────────> API   → 200 { diff_scan: { …artifacts… } }
(each GET is short; the SDK retries on 202 until 200)
What changed in the code
  • src/commands/scan/fetch-diff-scan.mts — this is the main change. It used to call the old endpoint directly with queryApiSafeJson. Now it gets the SDK client with setupSdk(), calls createOrgDiffScanFromIds(orgSlug, { before, after }) to create the diff-scan resource, then calls getDiffScanById(orgSlug, diffScanId, { cached: true }) to fetch the result. The SDK handles the 202 polling loop automatically, so the caller just gets the final 200. The create step is idempotent, so re-running over the same pair of scans returns the existing diff-scan id instead of making a new one.
  • src/commands/scan/output-diff-scan.mts — the response shape from the new endpoint is nested under data.diff_scan instead of being flat. So data.before is now data.diff_scan.before_full_scan, data.after is data.diff_scan.after_full_scan, and the dashboard link moved from data.diff_report_url to data.diff_scan.html_url. The code that prints the markdown table and the before/after scan details was updated to read from the nested shape.
  • data/command-api-requirements.json — the scan:diff command's quota dropped from 1 to 0 (the new endpoints are free) and its permissions now list diff-scans:create, diff-scans:list, and full-scans:list, matching the scopes the new endpoints require.
  • Tests — the unit tests for fetch-diff-scan, output-diff-scan, output-diff-scan-markdown, and cmd-scan-diff were updated to mock the new two-step flow and the nested response shape. 56 unit tests pass and the typecheck is clean on the changed files.
Field-name differences from the old endpoint

Two fields changed names between the old GetOrgDiffScan response and the new getDiffScanById response, and one was removed:

  • The dashboard report link was data.diff_report_url on the old endpoint. On the new endpoint it is data.diff_scan.html_url.
  • The before/after scans were data.before / data.after (flat). They are now data.diff_scan.before_full_scan / data.diff_scan.after_full_scan (nested under diff_scan).
  • The old endpoint returned a directDependenciesChanged boolean. The new endpoint does not have this field, so the markdown output no longer prints that line.

Refs SURF-743.

Migrate socket scan diff from the deprecated GET /full-scans/diff to POST /diff-scans/from-ids + GET /diff-scans/:id?cached=true. The cached path polls 202 until the immutable result is ready, avoiding the long-lived synchronous connection that caused 1200s timeouts under load (SURF-1487 family).

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Missing duplicate redirect on create
    • createOrgDiffScanFromIds now passes on_duplicate: 'redirect' so a repeated before/after pair reuses the existing diff scan instead of failing with 409.

Create PR

Or push these changes by commenting:

@cursor push 4ddf3dfbbb
Preview (4ddf3dfbbb)
diff --git a/packages/cli/src/commands/scan/fetch-diff-scan.mts b/packages/cli/src/commands/scan/fetch-diff-scan.mts
--- a/packages/cli/src/commands/scan/fetch-diff-scan.mts
+++ b/packages/cli/src/commands/scan/fetch-diff-scan.mts
@@ -27,7 +27,11 @@
   const sockSdk = sockSdkCResult.data
 
   const createResult = await handleApiCall<'createOrgDiffScanFromIds'>(
-    sockSdk.createOrgDiffScanFromIds(orgSlug, { before: id1, after: id2 }),
+    sockSdk.createOrgDiffScanFromIds(orgSlug, {
+      before: id1,
+      after: id2,
+      on_duplicate: 'redirect',
+    }),
     {
       description: 'a scan diff creation',
     },

diff --git a/packages/cli/test/unit/commands/scan/fetch-diff-scan.test.mts b/packages/cli/test/unit/commands/scan/fetch-diff-scan.test.mts
--- a/packages/cli/test/unit/commands/scan/fetch-diff-scan.test.mts
+++ b/packages/cli/test/unit/commands/scan/fetch-diff-scan.test.mts
@@ -183,6 +183,7 @@
     expect(mockSdk.createOrgDiffScanFromIds).toHaveBeenCalledWith('test-org', {
       before: 'scan-123',
       after: 'scan-456',
+      on_duplicate: 'redirect',
     })
     expect(mockSdk.getDiffScanById).toHaveBeenCalledWith(
       'test-org',
@@ -305,6 +306,7 @@
       expect(mockSdk.createOrgDiffScanFromIds).toHaveBeenCalledWith(orgSlug, {
         before: 'scan-1',
         after: 'scan-2',
+        on_duplicate: 'redirect',
       })
     }
   })
@@ -359,6 +361,7 @@
     expect(mockSdk.createOrgDiffScanFromIds).toHaveBeenCalledWith('test-org', {
       before: 'same-scan-id',
       after: 'same-scan-id',
+      on_duplicate: 'redirect',
     })
   })

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

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

Reviewed by Cursor Bugbot for commit f997f0d. Configure here.

Comment thread packages/cli/src/commands/scan/fetch-diff-scan.mts
Without on_duplicate=redirect, a re-run over an existing before/after pair hits 409 (the DB unique constraint fires) and the CLI fails. With redirect, the API returns 302 to the existing diff scan, the SDK follows it, and the CLI extracts the existing diff scan ID for the cached getDiffScanById poll.
@jdalton
John-David Dalton (jdalton) merged commit bfa604e into main Aug 18, 2026
4 checks passed
@jdalton
John-David Dalton (jdalton) deleted the jdalton/surf-743-cached-diff-scan branch August 18, 2026 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant