Skip to content

Commit 3cd0355

Browse files
leliaclaude
andcommitted
fix(ci): apply the pull request link to an already-compared scan pair
external_href is only honored while a diff scan is being created, so a re-run over the same before/after pair left the Dashboard report with no link back to its pull request. Send on_duplicate=update alongside it, which applies the link to the existing diff scan and answers 200 with the same envelope as a create. The 409-and-resolve path is retained for runs with no pull request context and for deployments that predate on_duplicate=update. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 635bc7d commit 3cd0355

4 files changed

Lines changed: 70 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
unless `--integration` is explicitly supplied.
2323
- Diff scans include the detected pull request or merge request URL as their
2424
external link, allowing Dashboard reports to retain their CI change context.
25+
Re-running a comparison over an already-compared scan pair now applies the
26+
link to the existing diff scan instead of leaving that report unassociated.
2527

2628
## 2.7.0
2729

socketsecurity/core/__init__.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,6 +1633,14 @@ def get_diff_scan_artifacts(
16331633
}
16341634
if external_href:
16351635
create_params["external_href"] = external_href
1636+
# external_href is only honored while a diff scan is being created,
1637+
# so re-running a comparison over an already-compared scan pair
1638+
# would otherwise leave the Dashboard report with no link back to
1639+
# the pull request. on_duplicate=update applies the link to the
1640+
# existing resource and answers 200 with the same {"diff_scan": ...}
1641+
# envelope as a create. Notably it is not on_duplicate=redirect,
1642+
# whose 302 the SDK follows into a GET without cached=true.
1643+
create_params["on_duplicate"] = "update"
16361644
try:
16371645
result = self.sdk.diffscans.create_from_ids(self.config.org_slug, create_params)
16381646
diff_scan = result.get("diff_scan") or {}
@@ -1641,11 +1649,14 @@ def get_diff_scan_artifacts(
16411649
if error.status_code != 409:
16421650
raise
16431651

1644-
# Do not use on_duplicate=redirect here. The SDK follows that 302
1645-
# automatically with a GET that lacks cached=true, which can leave
1646-
# the connection idle while an existing diff scan is still computing.
1647-
# Resolve the duplicate resource explicitly so every result fetch
1648-
# continues through the bounded cached polling path below.
1652+
# Reached without on_duplicate=update (no pull request context to
1653+
# attach) and against deployments that predate it and still answer
1654+
# 409 regardless. Do not switch this to on_duplicate=redirect: the
1655+
# SDK follows that 302 automatically with a GET that lacks
1656+
# cached=true, which can leave the connection idle while an existing
1657+
# diff scan is still computing. Resolve the duplicate resource
1658+
# explicitly so every result fetch continues through the bounded
1659+
# cached polling path below.
16491660
existing = self.sdk.diffscans.list(
16501661
self.config.org_slug,
16511662
params={

tests/core/test_diff_scan_polling.py

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ def test_duplicate_conflict_uses_cached_polling(core, diff_scan_get_response):
8787
artifacts = core.get_diff_scan_artifacts("head", "new")
8888

8989
create_params = core.sdk.diffscans.create_from_ids.call_args.args[1]
90-
assert "on_duplicate" not in create_params
90+
# "redirect" is the unsafe value: its 302 is followed into an uncached GET.
91+
assert create_params.get("on_duplicate") != "redirect"
9192
core.sdk.diffscans.list.assert_called_once_with(
9293
core.config.org_slug,
9394
params={
@@ -160,13 +161,59 @@ def test_eager_list_artifacts_do_not_bypass_filtered_get(core, diff_scan_get_res
160161
params={"cached": "true", "omit_unchanged": "true"},
161162
)
162163

164+
163165
def test_diff_scan_is_associated_with_pull_request_url(core):
164166
external_href = "https://dev.azure.com/acme/platform/_git/widgets/pullrequest/17"
165167

166168
core.get_diff_scan_artifacts("head", "new", external_href=external_href)
167169

168170
create_params = core.sdk.diffscans.create_from_ids.call_args.args[1]
169171
assert create_params["external_href"] == external_href
172+
# Without this the link is dropped whenever the scan pair was compared before.
173+
assert create_params["on_duplicate"] == "update"
174+
175+
176+
def test_no_duplicate_handling_requested_without_a_pull_request_url(core):
177+
"""Runs with no PR context keep the plain 409-and-resolve path."""
178+
core.get_diff_scan_artifacts("head", "new")
179+
180+
create_params = core.sdk.diffscans.create_from_ids.call_args.args[1]
181+
assert "on_duplicate" not in create_params
182+
assert "external_href" not in create_params
183+
184+
185+
def test_updated_duplicate_is_polled_like_a_created_diff_scan(core, diff_scan_get_response):
186+
"""on_duplicate=update answers 200 with the create envelope, not a 409.
187+
188+
The existing scan must then flow through the same cached-polling path, and
189+
the duplicate-resolving list call must not be needed at all.
190+
"""
191+
core.sdk.diffscans.create_from_ids.return_value = {
192+
"diff_scan": {"id": "existing-diff-scan"}
193+
}
194+
195+
artifacts = core.get_diff_scan_artifacts(
196+
"head", "new", external_href="https://github.com/acme/widgets/pull/42"
197+
)
198+
199+
core.sdk.diffscans.list.assert_not_called()
200+
assert core.sdk.diffscans.get.call_args.args[1] == "existing-diff-scan"
201+
assert len(artifacts.added) > 0
202+
203+
204+
def test_link_falls_back_to_resolving_the_duplicate_on_older_deployments(core):
205+
"""Deployments predating on_duplicate=update still answer 409; keep working."""
206+
core.sdk.diffscans.create_from_ids.side_effect = APIFailure(
207+
"duplicate", status_code=409
208+
)
209+
core.sdk.diffscans.list.return_value = {"results": [{"id": "existing-diff-scan"}]}
210+
211+
artifacts = core.get_diff_scan_artifacts(
212+
"head", "new", external_href="https://github.com/acme/widgets/pull/42"
213+
)
214+
215+
assert core.sdk.diffscans.get.call_args.args[1] == "existing-diff-scan"
216+
assert len(artifacts.added) > 0
170217

171218

172219
def test_fallback_to_streaming_diff_on_failure(core):

tests/unit/test_socketcli.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
from socketsecurity import socketcli
66
from socketsecurity.core.classes import Diff, Package
7-
from socketsecurity.socketcli import build_license_artifact_payload, should_write_comment
7+
from socketsecurity.socketcli import (
8+
build_license_artifact_payload,
9+
should_write_comment,
10+
)
811

912
# ---------------------------------------------------------------------------
1013
# Exit-code-on-api-error (flag-only, non-breaking for 2.3.x).

0 commit comments

Comments
 (0)