chore: address second round of PR feedback (#2909)

Co-authored-by: openhands <openhands@all-hands.dev>

Author: enyst

.github/workflows/issue-duplicate-checker.yml

@@ -291,14 +291,27 @@ jobs:
                       );
                       const body = sections.join('\n').trim();
 
-                      const { data: comments } = await github.rest.issues.listComments({
-                        owner: context.repo.owner,
-                        repo: context.repo.repo,
-                        issue_number: issueNumber,
-                        per_page: 100,
-                      });
+                      let allComments = [];
+                      let page = 1;
+                      while (true) {
+                        const { data: comments } = await github.rest.issues.listComments({
+                          owner: context.repo.owner,
+                          repo: context.repo.repo,
+                          issue_number: issueNumber,
+                          per_page: 100,
+                          page,
+                        });
+                        if (!comments || comments.length === 0) {
+                          break;
+                        }
+                        allComments = allComments.concat(comments);
+                        if (comments.length < 100) {
+                          break;
+                        }
+                        page += 1;
+                      }
 
-                      const existing = comments.find((comment) => comment.body && comment.body.includes('<!-- openhands-duplicate-check '));
+                      const existing = allComments.find((comment) => comment.body && comment.body.includes('<!-- openhands-duplicate-check '));
                       if (existing) {
                         if (autoClose) {
                           await ensureCandidateLabelOnIssue();

.github/workflows/remove-duplicate-candidate-label.yml

@@ -15,6 +15,8 @@ jobs:
             github.event.issue.pull_request == null &&
             contains(github.event.issue.labels.*.name, 'duplicate-candidate') &&
             github.event.comment.user.type != 'Bot' &&
+            !endsWith(github.event.comment.user.login, '[bot]') &&
+            github.event.comment.user.login != 'all-hands-bot' &&
             !contains(github.event.comment.body, '<!-- openhands-duplicate-check') &&
             !contains(github.event.comment.body, '<!-- openhands-duplicate-veto')
         runs-on: ubuntu-latest

scripts/auto_close_duplicate_issues.py

@@ -71,10 +71,15 @@ def request_json(
         raise RuntimeError(
             f"{method} {path} failed with HTTP {exc.code}: {error_body}"
         ) from exc
+    except urllib.error.URLError as exc:
+        raise RuntimeError(f"{method} {path} failed: {exc}") from exc
 
     if not payload:
         return None
-    return json.loads(payload)
+    try:
+        return json.loads(payload)
+    except json.JSONDecodeError as exc:
+        raise RuntimeError(f"Failed to parse JSON from {path}: {exc}") from exc
 
 
 def parse_timestamp(value: str) -> datetime:
@@ -291,8 +296,12 @@ def main() -> int:
 
     summary: list[dict[str, Any]] = []
     for issue in list_open_issues(args.repository):
-        issue_number = int(issue["number"])
-        issue_created_at = parse_timestamp(issue["created_at"])
+        issue_number = issue.get("number")
+        issue_created_at_str = issue.get("created_at")
+        if issue_number is None or not issue_created_at_str:
+            continue
+        issue_number = int(issue_number)
+        issue_created_at = parse_timestamp(issue_created_at_str)
         if issue_created_at > cutoff:
             continue
 
@@ -303,12 +312,16 @@ def main() -> int:
         if latest_comment is None or canonical_issue_number is None:
             continue
 
-        comment_created_at = parse_timestamp(latest_comment["created_at"])
+        comment_created_at_str = latest_comment.get("created_at")
+        comment_id = latest_comment.get("id")
+        if not comment_created_at_str or comment_id is None:
+            continue
+        comment_created_at = parse_timestamp(comment_created_at_str)
         if comment_created_at > cutoff:
             continue
 
         author_id = user_id_from_item(issue)
-        reactions = list_comment_reactions(args.repository, int(latest_comment["id"]))
+        reactions = list_comment_reactions(args.repository, int(comment_id))
         author_thumbs_down = has_reaction_from_user(reactions, author_id, "-1")
         author_thumbs_up = has_reaction_from_user(reactions, author_id, "+1")
         if author_thumbs_down:
@@ -341,7 +354,8 @@ def main() -> int:
         newer_comments = [
             comment
             for comment in comments
-            if parse_timestamp(comment["created_at"]) > comment_created_at
+            if comment.get("created_at")
+            and parse_timestamp(comment["created_at"]) > comment_created_at
         ]
         if newer_comments:
             summary.append(

scripts/issue_duplicate_check_openhands.py

@@ -106,15 +106,17 @@ def request_json(
     try:
         with urllib.request.urlopen(request, timeout=60) as response:
             return json.load(response)
-    except json.JSONDecodeError as exc:
-        raise RuntimeError(
-            f"Failed to parse JSON from {method} {base_url}{path}: {exc}"
-        ) from exc
     except urllib.error.HTTPError as exc:
         error_body = exc.read().decode("utf-8", errors="replace")
         raise RuntimeError(
             f"{method} {base_url}{path} failed with HTTP {exc.code}: {error_body}"
         ) from exc
+    except json.JSONDecodeError as exc:
+        raise RuntimeError(
+            f"Failed to parse JSON from {method} {base_url}{path}: {exc}"
+        ) from exc
+    except urllib.error.URLError as exc:
+        raise RuntimeError(f"{method} {base_url}{path} failed: {exc}") from exc
 
 
 def fetch_issue(repository: str, issue_number: int) -> dict[str, Any]:

tests/cross/test_issue_duplicate_scripts.py

@@ -124,6 +124,44 @@ def test_parse_timestamp_reports_invalid_values():
         module.parse_timestamp("invalid")
 
 
+def test_auto_close_request_json_reports_urlerror(monkeypatch):
+    module = load_module("auto_close_duplicate_issues.py")
+
+    monkeypatch.setattr(module, "github_headers", lambda: {})
+    monkeypatch.setattr(
+        module.urllib.request,
+        "urlopen",
+        lambda *args, **kwargs: (_ for _ in ()).throw(
+            module.urllib.error.URLError("boom")
+        ),
+    )
+
+    with pytest.raises(RuntimeError, match="GET /test failed"):
+        module.request_json("/test")
+
+
+def test_auto_close_request_json_reports_invalid_json(monkeypatch):
+    module = load_module("auto_close_duplicate_issues.py")
+    monkeypatch.setattr(module, "github_headers", lambda: {})
+
+    class DummyResponse:
+        def __enter__(self):
+            return self
+
+        def __exit__(self, exc_type, exc, tb):
+            return False
+
+        def read(self):
+            return b"not-json"
+
+    monkeypatch.setattr(
+        module.urllib.request, "urlopen", lambda *args, **kwargs: DummyResponse()
+    )
+
+    with pytest.raises(RuntimeError, match="Failed to parse JSON from /test"):
+        module.request_json("/test")
+
+
 def test_has_reaction_from_user_ignores_missing_user_ids():
     module = load_module("auto_close_duplicate_issues.py")
     reactions = [
@@ -382,6 +420,66 @@ def test_auto_close_main_closes_old_duplicate(monkeypatch, capsys):
     assert closed == [("OpenHands/agent-sdk", 123, 45, False)]
 
 
+def test_auto_close_main_skips_malformed_issue_data(monkeypatch, capsys):
+    module = load_module("auto_close_duplicate_issues.py")
+
+    monkeypatch.setattr(
+        module,
+        "parse_args",
+        lambda: argparse.Namespace(
+            repository="OpenHands/agent-sdk", close_after_days=3, dry_run=False
+        ),
+    )
+    monkeypatch.setattr(
+        module, "list_open_issues", lambda repository: [{"number": 123}]
+    )
+
+    assert module.main() == 0
+
+    summary = json.loads(capsys.readouterr().out)
+    assert summary == {"repository": "OpenHands/agent-sdk", "results": []}
+
+
+def test_auto_close_main_skips_malformed_duplicate_comment(monkeypatch, capsys):
+    module = load_module("auto_close_duplicate_issues.py")
+    now = datetime.now(UTC)
+    old_timestamp = iso_timestamp(now - timedelta(days=5))
+    issue = {
+        "number": 123,
+        "created_at": old_timestamp,
+        "labels": [{"name": module.DUPLICATE_CANDIDATE_LABEL}],
+        "user": {"id": 7},
+    }
+    comments = [
+        {
+            "body": "<!-- openhands-duplicate-check canonical=45 auto-close=true -->",
+            "created_at": old_timestamp,
+        }
+    ]
+
+    monkeypatch.setattr(
+        module,
+        "parse_args",
+        lambda: argparse.Namespace(
+            repository="OpenHands/agent-sdk", close_after_days=3, dry_run=False
+        ),
+    )
+    monkeypatch.setattr(module, "list_open_issues", lambda repository: [issue])
+    monkeypatch.setattr(
+        module, "list_issue_comments", lambda repository, number: comments
+    )
+    monkeypatch.setattr(
+        module,
+        "close_issue_as_duplicate",
+        lambda *args, **kwargs: pytest.fail("close_issue_as_duplicate should not run"),
+    )
+
+    assert module.main() == 0
+
+    summary = json.loads(capsys.readouterr().out)
+    assert summary == {"repository": "OpenHands/agent-sdk", "results": []}
+
+
 def test_auto_close_main_removes_label_when_newer_comment_exists(monkeypatch, capsys):
     module = load_module("auto_close_duplicate_issues.py")
     now = datetime.now(UTC)
@@ -500,6 +598,21 @@ def test_normalize_result_promotes_actionable_duplicates():
     assert normalized["summary"] == "duplicate summary"
 
 
+def test_issue_duplicate_request_json_reports_urlerror(monkeypatch):
+    module = load_module("issue_duplicate_check_openhands.py")
+
+    monkeypatch.setattr(
+        module.urllib.request,
+        "urlopen",
+        lambda *args, **kwargs: (_ for _ in ()).throw(
+            module.urllib.error.URLError("boom")
+        ),
+    )
+
+    with pytest.raises(RuntimeError, match="GET https://example.test/path failed"):
+        module.request_json("https://example.test", "/path")
+
+
 def test_normalize_result_lowercases_classification():
     module = load_module("issue_duplicate_check_openhands.py")
     normalized = module.normalize_result(