Skip to content

fix(purl): expose fail-open batch params and harden dedupe - #98

Merged
lelia merged 8 commits into
mainfrom
lelia/sdk-purl-post-bug
Aug 5, 2026
Merged

fix(purl): expose fail-open batch params and harden dedupe#98
lelia merged 8 commits into
mainfrom
lelia/sdk-purl-post-bug

Conversation

@lelia

@lelialelia commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

What & why

purl.post() inherited the batch purl API's fail-open behavior with no way to opt out: input purls whose resolution/analysis hasn't finished are silently omitted from the response, so a caller can't tell "this version is clean" from "this version was dropped." This bit socket-basics core-tool-watch (2026-07-29): a Dependabot bump to socketdev==3.3.0 was scored, its row was silently omitted, and the fail-closed guard turned that into a red build across main.

Changes (all backward-compat)

  • First-class typed params on purl.post(): poll, timeout_sec (→ timeoutSec), alerts, purl_errors (→ purlErrors). None = omit the param, so existing callers keep the server's fail-open default. **kwargs passthrough retained.
  • Docstring documenting the fail-open semantics, the "omitted ≈ no-data without alerts=true" gotcha, and the synthetic pendingScan/notFound alert types.
  • Dedupe hardeningDedupe.consolidate_and_merge_alerts now uses .get() for key/type/severity/action (identity tuples + consolidated dict), so synthetic status rows (built server-side from a minimal {type, key} base) no longer raise KeyError.
  • strict=True mode — compares requested component purls against returned inputPurl/purl and raises the new APIPartialResponse(missing=[...]) when inputs are absent; a first-class "partial batch" signal even without alerts=true.
  • Version 3.3.0 → 3.4.2 (version.py, pyproject.toml, uv.lock).

Tests

5 new unit tests (query-string per param, unset-param omission, synthetic pendingScan NDJSON parse, strict raise + pass). Full unit suite: 127 passed, 1 skipped.

Release plan

Publish one v3.4.2 release after this PR merges. It bundles the unreleased changes from #99 and #101 with this PR instead of publishing 3.4.0, 3.4.1, and 3.4.2 separately.

Todos

Fixes CE-360


Note

Medium Risk
Changes core batch purl client behavior and dedupe paths used by all purl scoring; opt-in strict mode can break callers that relied on silent omissions, but defaults preserve fail-open semantics.

Overview
purl.post() now exposes first-class query options for the batch API’s fail-open behavior: poll, timeout_sec, alerts, and purl_errors (omitted when None so defaults stay unchanged). A new strict=True mode compares requested component purl strings to returned inputPurl/purl (and purlError stream values) and raises APIPartialResponse with a missing list when the response drops inputs.

NDJSON handling splits purlError/summary stream records from artifact rows so errors are not deduped away; Dedupe uses .get() on alert fields so synthetic pendingScan/notFound rows no longer trigger KeyError. Version bumps to 3.4.2 with expanded unit tests.

Reviewed by Cursor Bugbot for commit 1623690. Configure here.

purl.post() defaulted to the batch API's fail-open behavior with no way to
opt out: unresolved input purls are silently omitted from the response, so
callers could not tell "clean" from "dropped". Add typed poll/timeout_sec/
alerts/purl_errors params (None => omit, preserving the fail-open default for
existing callers) plus a strict=True guard that raises APIPartialResponse when
requested purls are missing from the response.
Also harden Dedupe.consolidate_and_merge_alerts to use .get() for
key/type/severity/action so synthetic pendingScan/notFound status rows (built
server-side from a minimal {type, key} base) no longer raise KeyError.
Bump 3.3.0 -> 3.4.0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
@lelia
lelia requested a review from a team as a code ownerAugust 5, 2026 03:52
@lelia
leliatemporarily deployed to socket-firewall August 5, 2026 03:52 — with GitHub Actions Inactive
@github-actions

github-actionsBot 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.dev7

@lelialelia changed the title fix(purl): expose fail-open batch params and harden dedupe (CE-360)fix(purl): expose fail-open batch params and harden dedupeAug 5, 2026
@lelia

lelia commented Aug 5, 2026

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursor

cursorBot commented Aug 5, 2026

Copy link
Copy Markdown

Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings.

@lelia
leliatemporarily deployed to socket-firewall August 5, 2026 04:55 — with GitHub Actions Inactive
@lelia
leliatemporarily deployed to socket-firewall August 5, 2026 19:14 — with GitHub Actions Inactive
@flowstate

Copy link
Copy Markdown
Contributor

ai@cursor: Two things before merge. Everything else looks good — unit suite is green (137 passed / 1 skipped) and the dedupe hardening does what it says on the tin.

strict=True compares purls by exact string match

_raise_on_missing builds a set from the returned inputPurl/purl values and does plain membership testing against the requested purl strings, so it assumes the server echoes back byte-identical purls. If the API canonicalizes at all, strict mode raises on packages that were actually returned:

requested: pkg:npm/%40scope/pkg@1.0.0
echoed: pkg:npm/@scope/pkg@1.0.0
-> APIPartialResponse: missing=['pkg:npm/%40scope/pkg@1.0.0']
(the server did return a row for this package)

Scoped npm purls appear in both the %40scope and @scope forms in the wild, and qualifier ordering is another way this can drift.

Since strict mode exists to fail closed, a false positive turns a clean scan into a hard build failure — the same breakage that motivated this PR, just inverted. This is a question about the API contract rather than about the code, so it needs your read: is byte-exact echo of inputPurl guaranteed? If it is, worth stating in the docstring and this is a non-issue. If it isn't, the comparison needs to normalize both sides before diffing.

APIPartialResponse reports itself as non-retryable

It inherits the base is_transient_error(), which keys off status_code. With no HTTP status that's None, so it returns False — even though a partial batch is by definition the failure that clears once analysis finishes.

The other two no-status exceptions in this file already override it for exactly this reason:

classAPITimeout(APIFailure):
defis_transient_error(self) ->bool:
# No HTTP status: the request timed out client-side, so a retry may succeed.returnTrueclassAPIConnectionError(APIFailure):
defis_transient_error(self) ->bool:
# No HTTP status: the connection was dropped/reset mid-request, so a retry may succeed.returnTrue

So APIPartialResponse is the odd one out in a pattern the file already establishes, and the fix is a three-line override.

I'd do it here rather than as a follow-up, for three reasons:

  • This class ships for the first time in this PR. Once released, is_transient_error() == False is observable behavior. Flipping it later silently changes control flow inside every caller's retry loop with no signature change to signal it — the kind of change that needs a version bump and a changelog entry to land honestly. Right now it's free.
  • The retry-classification contract is being actively defined (Add transient-error classification to APIFailure #93). A new subclass that answers it wrong is also the one most likely to get copied by the next subclass, so the inconsistency compounds rather than sitting still.
  • There is already a consumer pattern branching on it. The diff-scan poll loop in Fix intermittent connection resets on scan comparison by polling the diff-scans endpoints socket-python-cli#284 does if not error.is_transient_error(): raise. Anything that later catches a partial batch in a loop shaped like that will hard-fail on the one error guaranteed to clear by waiting — and it'll look like a flaky backend rather than a misclassification, which is an expensive way to find out.

@lelia

lelia commented Aug 5, 2026

Copy link
Copy Markdown
ContributorAuthor

Thanks — both are legitimate public-contract questions, and I agree they should be resolved intentionally before this class ships. I pushed 7bdb34c with the design follow-ups.

Some motivation/context: this PR came from an internal Socket Basics core-tool-watch workflow, not a customer report. A newly published package version was silently omitted while analysis was still pending, and the workflow's existing fail-closed completeness guard went red. That exposed the broader issue: the batch PURL endpoint defaults fail-open, while the SDK did not expose the server's polling/status controls or offer a first-class completeness assertion. The planned rollout is SDK first, then explicit companion changes in the Python CLI and Basics.

Exact PURL matching

Agreed that the guarantee needed to be stated and regression-tested in this PR. The API contract defines inputPurl as the original unmodified PURL input string before normalization. We therefore kept exact matching deliberately instead of adding client-side normalization, which could change caller identity or create a second canonicalization contract in the SDK.

The updated docstring now explains that contract, and the new regression test covers the concrete %40scope request / canonical @scope response case: strict=True matches the exact inputPurl and does not report a false omission.

Retry classification

I agree with the point that the behavior should be explicit now, rather than accidentally inherited and changed after release. Where I'm pushing back is on APIPartialResponse being transient by definition.

The motivating omission happened because analysis was pending, but strict=True only knows that an HTTP 200 response omitted an input. The same signal can represent pending analysis, notFound, a malformed/unresolvable PURL, or a response-contract failure. Some of those may clear; others are permanent. Marking every partial response transient would cause generic retry loops to repeat an entire batch without knowing why it was incomplete or whether another attempt can help.

For this endpoint, poll=True plus timeout_sec is the explicit bounded retry mechanism. alerts=True and purl_errors=True expose pending/not-found/error reasons. This also matches the intended Basics adoption: bounded server polling + status rows + strict=True, where strictness is the final completeness assertion rather than the polling loop itself.

I added an explicit APIPartialResponse.is_transient_error() -> False override, documentation explaining the distinction, and a unit test. So we are taking the review concern about accidental public behavior, while intentionally not applying the proposed True classification.

The same commit bumps the package to 3.4.2 and regenerates uv.lock, since #99 is being tracked as 3.4.1.

# Conflicts:
#	pyproject.toml
#	socketdev/version.py
#	uv.lock
@lelia
leliatemporarily deployed to socket-firewall August 5, 2026 20:34 — with GitHub Actions Inactive
@lelia

lelia commented Aug 5, 2026

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadsocketdev/purl/__init__.py Outdated
@lelia

lelia commented Aug 5, 2026

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@lelia
leliadeployed to socket-firewall August 5, 2026 20:46 — with GitHub Actions Active

@cursorcursorBot 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.

✅ 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 1623690. Configure here.

@lelia
lelia merged commit 8ad17ae into mainAug 5, 2026
13 checks passed
Sign up for freeto 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.

3 participants

@lelia@flowstate@philgran