Skip to content

Commit 6e31308

Browse files
billxinlibillxinli
committed
feat(ignore): adding event logger for ignored comments
1 parent 29e2c61 commit 6e31308

File tree

4 files changed

+66
-80
lines changed

4 files changed

+66
-80
lines changed

‎socketsecurity/core/scm/github.py‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -211,20 +211,6 @@ def post_reaction(self, comment_id: int) -> None:
211211
base_url=self.config.api_url
212212
)
213213

214-
def post_eyes_reaction(self, comment_id: int) -> None:
215-
path = f"repos/{self.config.owner}/{self.config.repository}/issues/comments/{comment_id}/reactions"
216-
payload = json.dumps({"content": "eyes"})
217-
try:
218-
self.client.request(
219-
path=path,
220-
payload=payload,
221-
method="POST",
222-
headers=self.config.headers,
223-
base_url=self.config.api_url
224-
)
225-
except Exception as error:
226-
log.warning(f"Failed to add eyes reaction to comment {comment_id}: {error}")
227-
228214
def comment_reaction_exists(self, comment_id: int) -> bool:
229215
path = f"repos/{self.config.owner}/{self.config.repository}/issues/comments/{comment_id}/reactions"
230216
try:

‎socketsecurity/core/scm/gitlab.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,8 @@ def update_comment(self, body: str, comment_id: str) -> None:
220220
base_url=self.config.api_url
221221
)
222222

223-
def has_eyes_reaction(self, comment_id: int) -> bool:
224-
"""Best-effort check for 'eyes' award emoji on a MR note."""
223+
def has_thumbsup_reaction(self, comment_id: int) -> bool:
224+
"""Best-effort check for 'thumbsup' award emoji on a MR note."""
225225
if not self.config.mr_project_id or not self.config.mr_iid:
226226
return False
227227
path = f"projects/{self.config.mr_project_id}/merge_requests/{self.config.mr_iid}/notes/{comment_id}/award_emoji"
@@ -232,7 +232,7 @@ def has_eyes_reaction(self, comment_id: int) -> bool:
232232
base_url=self.config.api_url
233233
)
234234
for emoji in response.json():
235-
if emoji.get("name") == "eyes":
235+
if emoji.get("name") == "thumbsup":
236236
return True
237237
except Exception as e:
238238
log.debug(f"Could not check award emoji for note {comment_id} (best effort): {e}")
@@ -345,26 +345,32 @@ def set_commit_status(self, state: str, description: str, target_url: str = '')
345345
except Exception as e:
346346
log.error(f"Failed to set commit status: {e}")
347347

348-
def post_eyes_reaction(self, comment_id: int) -> None:
349-
"""Best-effort: add 'eyes' award emoji to a MR note. The token may lack permission."""
348+
def post_thumbsup_reaction(self, comment_id: int) -> None:
349+
"""Best-effort: add 'thumbsup' award emoji to a MR note."""
350350
if not self.config.mr_project_id or not self.config.mr_iid:
351351
return
352352
path = f"projects/{self.config.mr_project_id}/merge_requests/{self.config.mr_iid}/notes/{comment_id}/award_emoji"
353353
try:
354354
headers = {**self.config.headers, "Content-Type": "application/json"}
355355
self._request_with_fallback(
356356
path=path,
357-
payload=json.dumps({"name": "eyes"}),
357+
payload=json.dumps({"name": "thumbsup"}),
358358
method="POST",
359359
headers=headers,
360360
base_url=self.config.api_url
361361
)
362362
except Exception as e:
363-
log.debug(f"Could not add eyes emoji to note {comment_id} (best effort): {e}")
363+
log.debug(f"Could not add thumbsup emoji to note {comment_id} (best effort): {e}")
364+
365+
def handle_ignore_reactions(self, comments: dict) -> None:
366+
for comment in comments.get("ignore", []):
367+
if "SocketSecurity ignore" in comment.body and not self.has_thumbsup_reaction(comment.id):
368+
self.post_thumbsup_reaction(comment.id)
364369

365370
def remove_comment_alerts(self, comments: dict):
366371
security_alert = comments.get("security")
367372
if security_alert is not None:
368373
# Type narrowing: after None check, mypy knows this is Comment
369374
new_body = Comments.process_security_comment(security_alert, comments)
375+
self.handle_ignore_reactions(comments)
370376
self.update_comment(new_body, str(security_alert.id))

‎socketsecurity/socketcli.py‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -482,12 +482,12 @@ def main_code():
482482
log.debug(f"Flow decision: scm={scm is not None}, force_diff_mode={force_diff_mode}, force_api_mode={force_api_mode}, enable_diff={config.enable_diff}")
483483

484484
def _is_unprocessed(c):
485-
"""Check if an ignore comment has not yet been marked with 'eyes' reaction.
486-
For GitHub, reactions.eyes is already in the comment response (no extra call).
487-
For GitLab, has_eyes_reaction() makes a lazy API call per comment."""
488-
if getattr(c, "reactions", {}).get("eyes"):
485+
"""Check if an ignore comment has not yet been marked with '+1' reaction.
486+
For GitHub, reactions['+1'] is already in the comment response (no extra call).
487+
For GitLab, has_thumbsup_reaction() makes a lazy API call per comment."""
488+
if getattr(c, "reactions", {}).get("+1"):
489489
return False
490-
if hasattr(scm, "has_eyes_reaction") and scm.has_eyes_reaction(c.id):
490+
if hasattr(scm, "has_thumbsup_reaction") and scm.has_thumbsup_reaction(c.id):
491491
return False
492492
return True
493493

@@ -502,11 +502,8 @@ def _is_unprocessed(c):
502502

503503
comments = scm.get_comments_for_pr()
504504

505-
log.debug("Removing comment alerts")
506-
scm.remove_comment_alerts(comments)
507-
508-
# Emit telemetry only for ignore comments not yet marked with 'eyes' reaction.
509-
# Process each comment individually so the comment author is recorded per event.
505+
# Emit telemetry for ignore comments before +1 reaction is added.
506+
# The +1 reaction (added by remove_comment_alerts) serves as the "processed" marker.
510507
if "ignore" in comments:
511508
unprocessed = [c for c in comments["ignore"] if _is_unprocessed(c)]
512509
if unprocessed:
@@ -539,13 +536,11 @@ def _is_unprocessed(c):
539536
if events:
540537
log.debug(f"Ignore telemetry: {len(events)} events to send")
541538
client.post_telemetry_events(org_slug, events)
542-
543-
# Mark as processed with eyes reaction
544-
if hasattr(scm, "post_eyes_reaction"):
545-
for c in unprocessed:
546-
scm.post_eyes_reaction(c.id)
547539
except Exception as e:
548540
log.warning(f"Failed to send ignore telemetry: {e}")
541+
542+
log.debug("Removing comment alerts")
543+
scm.remove_comment_alerts(comments)
549544

550545
elif scm is not None and scm.check_event_type() != "comment" and not force_api_mode:
551546
log.info("Push initiated flow")
@@ -618,10 +613,9 @@ def _is_unprocessed(c):
618613
if events:
619614
client.post_telemetry_events(org_slug, events)
620615

621-
# Mark ignore comments as processed
622-
if hasattr(scm, "post_eyes_reaction"):
623-
for c in unprocessed_ignore:
624-
scm.post_eyes_reaction(c.id)
616+
# Mark ignore comments as processed with +1 reaction
617+
if hasattr(scm, "handle_ignore_reactions"):
618+
scm.handle_ignore_reactions(comments)
625619
except Exception as e:
626620
log.warning(f"Failed to send ignore telemetry: {e}")
627621

‎tests/unit/test_ignore_telemetry_filtering.py‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,55 @@
1-
"""Tests for the eyes-reaction dedup logic used to filter ignore comments for telemetry."""
1+
"""Tests for the +1 reaction dedup logic used to filter ignore comments for telemetry."""
22

33
from unittest.mock import Mock
44

55
from socketsecurity.core.classes import Comment
66
from socketsecurity.core.scm_comments import Comments
77

88

9-
def _make_comment(body: str, eyes: int = 0, comment_id: int = 1, user: dict | None = None) -> Comment:
9+
def _make_comment(body: str, thumbs_up: int = 0, comment_id: int = 1, user: dict | None = None) -> Comment:
1010
return Comment(
1111
id=comment_id,
1212
body=body,
1313
body_list=body.split("\n"),
14-
reactions={"eyes": eyes, "+1": 1},
14+
reactions={"+1": thumbs_up},
1515
user=user or {"login": "test-user", "id": 123},
1616
)
1717

1818

1919
def _filter_unprocessed(comments: list[Comment], scm=None) -> list[Comment]:
2020
"""Mirrors the _is_unprocessed logic in socketcli.py."""
2121
def _is_unprocessed(c):
22-
if getattr(c, "reactions", {}).get("eyes"):
22+
if getattr(c, "reactions", {}).get("+1"):
2323
return False
24-
if hasattr(scm, "has_eyes_reaction") and scm.has_eyes_reaction(c.id):
24+
if hasattr(scm, "has_thumbsup_reaction") and scm.has_thumbsup_reaction(c.id):
2525
return False
2626
return True
2727

2828
return [c for c in comments if _is_unprocessed(c)]
2929

3030

3131
class TestUnprocessedIgnoreFiltering:
32-
def test_returns_comments_without_eyes(self):
32+
def test_returns_comments_without_thumbsup(self):
3333
comments = [
34-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=0, comment_id=1),
35-
_make_comment("SocketSecurity ignore npm/express@4.18.2", eyes=0, comment_id=2),
34+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=0, comment_id=1),
35+
_make_comment("SocketSecurity ignore npm/express@4.18.2", thumbs_up=0, comment_id=2),
3636
]
3737
result = _filter_unprocessed(comments)
3838
assert len(result) == 2
3939

40-
def test_excludes_comments_with_eyes(self):
40+
def test_excludes_comments_with_thumbsup(self):
4141
comments = [
42-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=1, comment_id=1),
43-
_make_comment("SocketSecurity ignore npm/express@4.18.2", eyes=0, comment_id=2),
42+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=1, comment_id=1),
43+
_make_comment("SocketSecurity ignore npm/express@4.18.2", thumbs_up=0, comment_id=2),
4444
]
4545
result = _filter_unprocessed(comments)
4646
assert len(result) == 1
4747
assert result[0].id == 2
4848

4949
def test_returns_empty_when_all_processed(self):
5050
comments = [
51-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=1, comment_id=1),
52-
_make_comment("SocketSecurity ignore-all", eyes=2, comment_id=2),
51+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=1, comment_id=1),
52+
_make_comment("SocketSecurity ignore-all", thumbs_up=2, comment_id=2),
5353
]
5454
result = _filter_unprocessed(comments)
5555
assert len(result) == 0
@@ -66,58 +66,58 @@ def test_handles_empty_reactions_dict(self):
6666
result = _filter_unprocessed([c])
6767
assert len(result) == 1
6868

69-
def test_handles_reactions_with_eyes_zero(self):
70-
c = _make_comment("SocketSecurity ignore npm/foo@1.0.0", eyes=0, comment_id=1)
69+
def test_handles_reactions_with_thumbsup_zero(self):
70+
c = _make_comment("SocketSecurity ignore npm/foo@1.0.0", thumbs_up=0, comment_id=1)
7171
result = _filter_unprocessed([c])
7272
assert len(result) == 1
7373

7474

7575
class TestUnprocessedIgnoreFilteringWithScmFallback:
76-
"""Tests for the has_eyes_reaction fallback path (GitLab)."""
76+
"""Tests for the has_thumbsup_reaction fallback path (GitLab)."""
7777

7878
def test_scm_fallback_excludes_processed_comments(self):
79-
"""When inline reactions.eyes is 0 but scm says it has eyes, exclude it."""
79+
"""When inline reactions['+1'] is 0 but scm says it has thumbsup, exclude it."""
8080
comments = [
81-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=0, comment_id=1),
82-
_make_comment("SocketSecurity ignore npm/express@4.18.2", eyes=0, comment_id=2),
81+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=0, comment_id=1),
82+
_make_comment("SocketSecurity ignore npm/express@4.18.2", thumbs_up=0, comment_id=2),
8383
]
8484
scm = Mock()
85-
scm.has_eyes_reaction = Mock(side_effect=lambda cid: cid == 1)
85+
scm.has_thumbsup_reaction = Mock(side_effect=lambda cid: cid == 1)
8686

8787
result = _filter_unprocessed(comments, scm=scm)
8888
assert len(result) == 1
8989
assert result[0].id == 2
9090

91-
def test_scm_fallback_not_called_when_inline_eyes_present(self):
92-
"""When inline reactions.eyes is truthy, scm.has_eyes_reaction should not be called."""
91+
def test_scm_fallback_not_called_when_inline_thumbsup_present(self):
92+
"""When inline reactions['+1'] is truthy, scm.has_thumbsup_reaction should not be called."""
9393
comments = [
94-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=1, comment_id=1),
94+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=1, comment_id=1),
9595
]
9696
scm = Mock()
97-
scm.has_eyes_reaction = Mock(return_value=False)
97+
scm.has_thumbsup_reaction = Mock(return_value=False)
9898

9999
result = _filter_unprocessed(comments, scm=scm)
100100
assert len(result) == 0
101-
scm.has_eyes_reaction.assert_not_called()
101+
scm.has_thumbsup_reaction.assert_not_called()
102102

103-
def test_scm_without_has_eyes_reaction_skips_fallback(self):
104-
"""When scm doesn't have has_eyes_reaction (e.g. GitHub), only inline check runs."""
103+
def test_scm_without_has_thumbsup_reaction_skips_fallback(self):
104+
"""When scm doesn't have has_thumbsup_reaction (e.g. GitHub), only inline check runs."""
105105
comments = [
106-
_make_comment("SocketSecurity ignore npm/foo@1.0.0", eyes=0, comment_id=1),
106+
_make_comment("SocketSecurity ignore npm/foo@1.0.0", thumbs_up=0, comment_id=1),
107107
]
108108
scm = Mock(spec=[]) # no methods at all
109109

110110
result = _filter_unprocessed(comments, scm=scm)
111111
assert len(result) == 1
112112

113113
def test_scm_fallback_returns_all_unprocessed(self):
114-
"""When scm says none have eyes, all are returned."""
114+
"""When scm says none have thumbsup, all are returned."""
115115
comments = [
116-
_make_comment("SocketSecurity ignore npm/foo@1.0.0", eyes=0, comment_id=1),
117-
_make_comment("SocketSecurity ignore npm/bar@2.0.0", eyes=0, comment_id=2),
116+
_make_comment("SocketSecurity ignore npm/foo@1.0.0", thumbs_up=0, comment_id=1),
117+
_make_comment("SocketSecurity ignore npm/bar@2.0.0", thumbs_up=0, comment_id=2),
118118
]
119119
scm = Mock()
120-
scm.has_eyes_reaction = Mock(return_value=False)
120+
scm.has_thumbsup_reaction = Mock(return_value=False)
121121

122122
result = _filter_unprocessed(comments, scm=scm)
123123
assert len(result) == 2
@@ -128,9 +128,9 @@ class TestUnprocessedIgnoreFilteringWithCommentsParsing:
128128

129129
def test_only_new_artifacts_are_parsed(self):
130130
comments = [
131-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=1, comment_id=1),
132-
_make_comment("SocketSecurity ignore npm/express@4.18.2", eyes=0, comment_id=2),
133-
_make_comment("SocketSecurity ignore npm/axios@1.6.0", eyes=0, comment_id=3),
131+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=1, comment_id=1),
132+
_make_comment("SocketSecurity ignore npm/express@4.18.2", thumbs_up=0, comment_id=2),
133+
_make_comment("SocketSecurity ignore npm/axios@1.6.0", thumbs_up=0, comment_id=3),
134134
]
135135
unprocessed = _filter_unprocessed(comments)
136136
unprocessed_comments = {"ignore": unprocessed}
@@ -143,8 +143,8 @@ def test_only_new_artifacts_are_parsed(self):
143143

144144
def test_ignore_all_from_unprocessed(self):
145145
comments = [
146-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=1, comment_id=1),
147-
_make_comment("SocketSecurity ignore-all", eyes=0, comment_id=2),
146+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=1, comment_id=1),
147+
_make_comment("SocketSecurity ignore-all", thumbs_up=0, comment_id=2),
148148
]
149149
unprocessed = _filter_unprocessed(comments)
150150
unprocessed_comments = {"ignore": unprocessed}
@@ -154,8 +154,8 @@ def test_ignore_all_from_unprocessed(self):
154154

155155
def test_no_unprocessed_means_no_telemetry(self):
156156
comments = [
157-
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", eyes=1, comment_id=1),
158-
_make_comment("SocketSecurity ignore npm/express@4.18.2", eyes=2, comment_id=2),
157+
_make_comment("SocketSecurity ignore npm/lodash@4.17.21", thumbs_up=1, comment_id=1),
158+
_make_comment("SocketSecurity ignore npm/express@4.18.2", thumbs_up=2, comment_id=2),
159159
]
160160
unprocessed = _filter_unprocessed(comments)
161161
assert len(unprocessed) == 0
@@ -228,7 +228,7 @@ def test_gitlab_author_populates_sender(self):
228228
c = Comment(
229229
id=1, body="SocketSecurity ignore npm/foo@1.0.0",
230230
body_list=["SocketSecurity ignore npm/foo@1.0.0"],
231-
reactions={"eyes": 0},
231+
reactions={"+1": 0},
232232
author={"username": "gitlab-dev", "id": 42},
233233
)
234234
event = _build_event(c, artifact_input="npm/foo@1.0.0")

0 commit comments

Comments
 (0)