feat(scan): serve socket scan view from cached results with 202 polling (SURF-742) - #1425
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
b784521 to
c57dc34
Compare
John-David Dalton (jdalton)
changed the title
|
Withdrawing — the cached scan-view behavior (SURF-742) will land as part of bumping socket-cli v1.x to the latest @socketsecurity/sdk (4.x) under SURF-1446, rather than a bespoke direct-API poll. Superseding this branch. |
John-David Dalton (jdalton)
changed the title
socket scan view from cached results with 202 polling (SURF-742)
Backports the cached scan-view behavior from main (e5fb60b, f6741d2) to the v1.x line. socket scan view (default and --json non-stream) now reads pre-computed results from the immutable scan store: fetchScan hits ...full-scans/{id}?cached=true, polls on 202 ({status:'processing'}) with exponential backoff and a 10 minute deadline, then parses the 200 ndjson. The --stream --json live-stream path is unchanged. Uses the direct API via a new queryApiSafeTextWithStatus helper (which surfaces the HTTP status so the 202 poll loop can see it) — no SDK dependency, so no @socketsecurity/sdk release is required.
c57dc34 to
800e362
Compare
There was a problem hiding this comment.
We should definitely validate this on real data before releasing, and also this seems preferable to live in the SDK, so all clients benefit from the improvement, but otherwise on target. Please break out additional tickets for the SDK if you think thats a reasonable direction to take and even consider landing it via that approach, but if you want to just get this out as an intermediate step that also seems fine.
| // callers only ever observe the final scan. | ||
| const path = `orgs/${orgSlug}/full-scans/${encodeURIComponent(scanId)}?cached=true` | ||
| const deadline = Date.now() + CACHED_POLL_TIMEOUT_MS | ||
| let delayMs = CACHED_POLL_INITIAL_DELAY_MS |
There was a problem hiding this comment.
Should double check this against what the actual full scan timeout is in depscan.
| // 200 carries the ndjson body, a 202 means the server enqueued a background | ||
| // job to compute them — poll with backoff until the results are ready, so | ||
| // callers only ever observe the final scan. | ||
| const path = `orgs/${orgSlug}/full-scans/${encodeURIComponent(scanId)}?cached=true` |
There was a problem hiding this comment.
Any reason this isn't implemented in the SDK and then just consumed in the CLI? Clients of that shouldn't need to implement this logic.
2 participants
What this does
socket scan viewshows the results of a full scan. Today it asks the API to compute those results from scratch every time. This change makes it read a pre-computed (cached) copy instead, which is faster and cheaper.It does that by adding
?cached=trueto the request. The server has two possible answers:{"status":"processing"}. When this happens the CLI waits a moment and asks again, backing off a little each time (1s, then 2s, … up to 10s, giving up after 10 minutes). The user never sees the "processing" state — they just wait a bit longer and get the finished scan.Which modes change: the default view and
--json(the non-streaming modes that already buffer the whole response). The live--stream --jsonpath is intentionally left untouched.Why it's safe to ship on its own
This is self-contained. It uses the CLI's own HTTP helper (
queryApiSafeTextWithStatus) and does not call the Socket SDK for this path, so it stays on the currently-pinned@socketsecurity/sdk(1.4.96) and needs no SDK release. CI is green on its own.The separate effort to move socket-cli
v1.xonto the latest SDK (4.x) is tracked as SURF-1446 — that is not required by, and does not block, this PR.Implementation detail & parity with
mainqueryApiSafeTextWithStatusinutils/api.mtsreturns{ status, text }so the caller can tell a 202 apart from a 200 (the existingqueryApiSafeTextswallowed the status).queryApiSafeTextnow delegates to it, so its behavior is unchanged for every other caller.fetchScanbuilds the?cached=truerequest, runs the 202→200 backoff loop, and parses the 200 ndjson via an exportedparseArtifactsNdjson.main's CLI already does cached scan reads (commitse5fb60b1+f6741d2d), adapted to the older v1.x layout. It deliberately does not use the SDK's cachedgetFullScan, because that method returns scan metadata, not the artifact listscan viewrenders.--stream --jsonstill calls the streaming full-scan method unchanged.Verification
tsgo,eslint,oxlint,biome: clean.vitest src/commands/scan/fetch-scan.test.mts: 5 tests vianockagainst the real HTTP path — 200 cache hit, 202→200 poll, 404 error, plusparseArtifactsNdjsonvalid/invalid line cases. No owned-module mocking.vitest src/utils/api.test.mts: 10 tests, no regression from thequeryApiSafeTextrefactor.Linear: SURF-742. Do not merge yet.