Skip to content

Commit 239c85f

Browse files
leliaclaude
andcommitted
docs: rewrite branch comments for the reader, not the author
Sweep of every comment this branch adds, against the fourth-wall skill: - A test docstring stated the scan type "(since 2.8.0)", which was already wrong after the renumber to 2.9.0 and would rot again on the next one. Version stamps in comments describe a debut rather than the behavior. - Two docstrings narrated the failure the old parser produced instead of the invariant that makes rsplit correct. A scoped name carrying its own "@" is the whole reason; the traceback it used to raise is not. - The "do NOT use on_duplicate=redirect" landmine was explained twice, in full, at both call sites. Kept at the 409 fallback, where the temptation to add it lives; the create site now just says what update does. - A test section header justified its own design to a reviewer ("swapping the call back ... fails them"). Restated as what the test actually pins. - "out of this branch" in the remote-URL regex reads as a git branch in this repo; it means the regex case. 642 passed, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent bd2a3c3 commit 239c85f

6 files changed

Lines changed: 17 additions & 23 deletions

File tree

‎socketsecurity/core/__init__.py‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,12 +1708,9 @@ def get_diff_scan_artifacts(
17081708
if external_href:
17091709
create_params["external_href"] = external_href
17101710
# external_href is only honored while a diff scan is being created,
1711-
# so re-running a comparison over an already-compared scan pair
1712-
# would otherwise leave the Dashboard report with no link back to
1713-
# the pull request. on_duplicate=update applies the link to the
1714-
# existing resource and answers 200 with the same {"diff_scan": ...}
1715-
# envelope as a create. Notably it is not on_duplicate=redirect,
1716-
# whose 302 the SDK follows into a GET without cached=true.
1711+
# so a re-run over an already-compared scan pair needs
1712+
# on_duplicate=update to apply the link to the existing resource. It
1713+
# answers 200 with the same {"diff_scan": ...} envelope as a create.
17171714
create_params["on_duplicate"] = "update"
17181715
try:
17191716
result = self.sdk.diffscans.create_from_ids(self.config.org_slug, create_params)
@@ -1723,14 +1720,13 @@ def get_diff_scan_artifacts(
17231720
if error.status_code != 409:
17241721
raise
17251722

1726-
# Reached without on_duplicate=update (no pull request context to
1727-
# attach) and against deployments that predate it and still answer
1728-
# 409 regardless. Do not switch this to on_duplicate=redirect: the
1729-
# SDK follows that 302 automatically with a GET that lacks
1730-
# cached=true, which can leave the connection idle while an existing
1731-
# diff scan is still computing. Resolve the duplicate resource
1732-
# explicitly so every result fetch continues through the bounded
1733-
# cached polling path below.
1723+
# Reached when there is no pull request context to attach, and on
1724+
# deployments that answer 409 regardless. Do NOT switch this to
1725+
# on_duplicate=redirect: the SDK follows that 302 automatically with
1726+
# a GET that lacks cached=true, which can leave the connection idle
1727+
# while an existing diff scan is still computing. Resolve the
1728+
# duplicate explicitly so every result fetch continues through the
1729+
# bounded cached polling path below.
17341730
existing = self.sdk.diffscans.list(
17351731
self.config.org_slug,
17361732
params={

‎socketsecurity/core/git_remote.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from urllib.parse import urlparse
1111

1212
# git@host:owner/repo - the scp-like syntax urlparse cannot handle. The negative
13-
# lookahead keeps scheme-prefixed URLs (https://, ssh://) out of this branch.
13+
# lookahead keeps scheme-prefixed URLs (https://, ssh://) out of this case.
1414
_SCP_LIKE_REMOTE = re.compile(r"^(?:[^@/]+@)?([^:/]+):(?!//)(.+)$")
1515

1616

‎socketsecurity/core/scm_comments.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,7 @@ def process_original_security_comment(
164164
details, _ = package.split("](")
165165
ecosystem, details = details.split("/", 1)
166166
ecosystem = ecosystem.lstrip("[")
167-
# Split from the right: a scoped name carries its own "@", so
168-
# split("@") unpacks into three parts and raises.
167+
# Split from the right: a scoped name carries its own "@".
169168
pkg_name, pkg_version = details.rsplit("@", 1)
170169
# ignore_all has to be checked outside the loop: an ignore-all
171170
# comment produces no ignore_commands, so a loop-internal check

‎tests/core/test_full_scan_outputs.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
"""What a full scan has to carry for each enabled output.
22
33
create_full_scan_with_report_url runs on every path with no baseline to compare
4-
against: API mode, and (since 2.8.0) SCM branch pipelines. Fetching the SBOM is
5-
the expensive part, so it is gated on the enabled outputs -- these pin which
6-
outputs need it.
4+
against: API mode and SCM branch pipelines. Fetching the SBOM is the expensive
5+
part, so it is gated on the enabled outputs -- these pin which outputs need it.
76
"""
87
import pytest
98
from socketdev.fullscans import FullScanParams

‎tests/unit/test_pr_comment_rendering.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ def test_ignore_all_collapses_to_the_no_alerts_body(self):
328328
assert "[View full report](https://socket.dev/report/legacy)" in new_body
329329

330330
def test_scoped_package_row_does_not_raise(self):
331-
"""A scoped name carries its own "@", so split("@") unpacked into three."""
331+
"""A scoped name carries its own "@", so the split must come from the right."""
332332
security = _make_comment(SCOPED_LEGACY_COMMENT)
333333
comments = {"security": security, "ignore": []}
334334

‎tests/unit/test_socketcli.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ def test_pr_context_provider_uses_integration_without_comment_adapter():
8080
#
8181
# Only a pull request or merge request event has a baseline, so every other
8282
# pipeline gets a full scan. These drive create_scm_scan against a recording
83-
# stub rather than asserting on the branch predicate, so swapping the call back
84-
# to create_new_diff fails them.
83+
# stub rather than asserting on a predicate, so they fail if the branch stops
84+
# reaching create_full_scan_with_report_url.
8585
# ---------------------------------------------------------------------------
8686

8787

0 commit comments

Comments
 (0)