From 12c8e080ffcad1f06c60215d396eaf03773c202e Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Sun, 14 Jun 2026 22:59:11 +0400 Subject: [PATCH] fix(ci): ignore resolved Codex inline findings --- .github/scripts/sync_codex_ok_labels.py | 54 ++++++++++++++++++- .../proposal.md | 19 +++++++ .../specs/github-automation/spec.md | 23 ++++++++ .../tasks.md | 6 +++ tests/unit/test_sync_codex_ok_labels.py | 54 +++++++++++++++++++ 5 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 openspec/changes/ignore-resolved-codex-inline-findings/proposal.md create mode 100644 openspec/changes/ignore-resolved-codex-inline-findings/specs/github-automation/spec.md create mode 100644 openspec/changes/ignore-resolved-codex-inline-findings/tasks.md diff --git a/.github/scripts/sync_codex_ok_labels.py b/.github/scripts/sync_codex_ok_labels.py index c7bd17c1e..0675517ba 100755 --- a/.github/scripts/sync_codex_ok_labels.py +++ b/.github/scripts/sync_codex_ok_labels.py @@ -373,8 +373,57 @@ def timeline_head_oid(node: dict[str, Any]) -> str | None: return None +def unresolved_review_comment_urls(repo: str, number: int) -> set[str]: + owner, name = repo.split("/", 1) + after: str | None = None + urls: set[str] = set() + + while True: + fields: dict[str, object] = {"owner": owner, "name": name, "number": number} + if after is not None: + fields["after"] = after + payload = graphql(PR_REVIEW_THREADS_QUERY, **fields) + pr = payload.get("data", {}).get("repository", {}).get("pullRequest") + if not isinstance(pr, dict): + raise GhError(f"{repo}#{number}: GraphQL did not return a pull request") + + threads = pr.get("reviewThreads") + if not isinstance(threads, dict): + raise GhError(f"{repo}#{number}: GraphQL did not return review threads") + nodes = threads.get("nodes", []) + if not isinstance(nodes, list): + raise GhError(f"{repo}#{number}: GraphQL did not return review thread nodes") + + for thread in nodes: + if not isinstance(thread, dict): + continue + if thread.get("isResolved") or thread.get("isOutdated"): + continue + comments = thread.get("comments") + comment_nodes = comments.get("nodes") if isinstance(comments, dict) else [] + if not isinstance(comment_nodes, list): + continue + for comment in comment_nodes: + if not isinstance(comment, dict): + continue + url = comment.get("url") + if isinstance(url, str): + urls.add(url) + + page_info = threads.get("pageInfo") + if not isinstance(page_info, dict) or not page_info.get("hasNextPage"): + break + end_cursor = page_info.get("endCursor") + if not isinstance(end_cursor, str) or not end_cursor: + break + after = end_cursor + + return urls + + def pull_review_comment_nodes(repo: str, number: int, *, head_sha: str) -> list[dict[str, Any]]: comments = paged_api(f"/repos/{repo}/pulls/{number}/comments") + unresolved_urls = unresolved_review_comment_urls(repo, number) nodes: list[dict[str, Any]] = [] for comment in comments: body = comment.get("body") @@ -385,6 +434,9 @@ def pull_review_comment_nodes(repo: str, number: int, *, head_sha: str) -> list[ review_id = comment.get("pull_request_review_id") if commit_id != head_sha and original_commit_id != head_sha: continue + html_url = comment.get("html_url") or comment.get("url") + if is_needs_work_codex_body(body) and html_url not in unresolved_urls: + continue user = comment.get("user") login = user.get("login") if isinstance(user, dict) else None nodes.append( @@ -393,7 +445,7 @@ def pull_review_comment_nodes(repo: str, number: int, *, head_sha: str) -> list[ "author": {"login": login} if isinstance(login, str) else None, "bodyText": body, "createdAt": comment.get("created_at"), - "url": comment.get("html_url") or comment.get("url"), + "url": html_url, "commit": {"oid": commit_id} if isinstance(commit_id, str) else None, "pullRequestReviewDatabaseId": review_id if isinstance(review_id, int) else None, } diff --git a/openspec/changes/ignore-resolved-codex-inline-findings/proposal.md b/openspec/changes/ignore-resolved-codex-inline-findings/proposal.md new file mode 100644 index 000000000..643d60e4a --- /dev/null +++ b/openspec/changes/ignore-resolved-codex-inline-findings/proposal.md @@ -0,0 +1,19 @@ +## Why + +The Codex review label synchronizer can keep `🤖 codex: needs work` on a PR +after a maintainer resolves an inline Codex finding. The unresolved-thread check +already knows resolved review threads are no longer blockers, but the timeline +classifier still merges the old inline review comment and treats it as current +needs-work evidence. + +## What Changes + +- Ignore inline Codex finding comments from resolved or outdated review threads + when computing current-head review state. +- Keep unresolved inline Codex finding comments as needs-work evidence. +- Add regression coverage for both resolved and unresolved inline findings. + +## Impact + +- GitHub automation only. +- No application runtime behavior changes. diff --git a/openspec/changes/ignore-resolved-codex-inline-findings/specs/github-automation/spec.md b/openspec/changes/ignore-resolved-codex-inline-findings/specs/github-automation/spec.md new file mode 100644 index 000000000..aa30a73ce --- /dev/null +++ b/openspec/changes/ignore-resolved-codex-inline-findings/specs/github-automation/spec.md @@ -0,0 +1,23 @@ +## MODIFIED Requirements + +### Requirement: Codex review label sync review-thread state + +The Codex label synchronization script MUST treat unresolved, non-outdated +Codex inline review findings on the current head as needs-work evidence. It +MUST NOT treat inline Codex findings from resolved or outdated review threads as +active needs-work evidence. + +#### Scenario: Resolved inline finding no longer blocks the ok label + +- **WHEN** a current-head inline Codex finding comment belongs to a resolved + review thread +- **AND** a clean current-head Codex review exists +- **THEN** the script does not classify that inline finding as active + needs-work evidence + +#### Scenario: Unresolved inline finding still blocks the ok label + +- **WHEN** a current-head inline Codex finding comment belongs to an unresolved, + non-outdated review thread +- **THEN** the script classifies that inline finding as active needs-work + evidence diff --git a/openspec/changes/ignore-resolved-codex-inline-findings/tasks.md b/openspec/changes/ignore-resolved-codex-inline-findings/tasks.md new file mode 100644 index 000000000..7681377be --- /dev/null +++ b/openspec/changes/ignore-resolved-codex-inline-findings/tasks.md @@ -0,0 +1,6 @@ +## Implementation + +- [x] Filter current-head inline Codex finding comments by unresolved review-thread URLs. +- [x] Preserve unresolved inline finding comments as needs-work evidence. +- [x] Add unit coverage for resolved and unresolved inline findings. +- [x] Validate the focused test and OpenSpec gates. diff --git a/tests/unit/test_sync_codex_ok_labels.py b/tests/unit/test_sync_codex_ok_labels.py index db5c68c78..1f5e535e6 100644 --- a/tests/unit/test_sync_codex_ok_labels.py +++ b/tests/unit/test_sync_codex_ok_labels.py @@ -211,3 +211,57 @@ def fail_apply(*_args: Any, **_kwargs: Any) -> tuple[str, ...]: captured = capsys.readouterr() assert result == 1 assert "Soju06/codex-lb#714: gh: HTTP 500 while writing labels" in captured.err + + +def test_resolved_inline_codex_finding_does_not_count_as_review_news( + monkeypatch: pytest.MonkeyPatch, +) -> None: + module = load_sync_module() + + monkeypatch.setattr( + module, + "paged_api", + lambda _path: [ + { + "body": "**P1 Badge** resolved finding", + "commit_id": "a" * 40, + "original_commit_id": "a" * 40, + "pull_request_review_id": 123, + "html_url": "https://github.test/review/resolved", + "created_at": "2026-06-14T00:00:00Z", + "user": {"login": "chatgpt-codex-connector"}, + } + ], + ) + monkeypatch.setattr(module, "unresolved_review_comment_urls", lambda *_args: set()) + + assert module.pull_review_comment_nodes("Soju06/codex-lb", 714, head_sha="a" * 40) == [] + + +def test_unresolved_inline_codex_finding_counts_as_review_news( + monkeypatch: pytest.MonkeyPatch, +) -> None: + module = load_sync_module() + url = "https://github.test/review/unresolved" + + monkeypatch.setattr( + module, + "paged_api", + lambda _path: [ + { + "body": "**P1 Badge** unresolved finding", + "commit_id": "a" * 40, + "original_commit_id": "a" * 40, + "pull_request_review_id": 123, + "html_url": url, + "created_at": "2026-06-14T00:00:00Z", + "user": {"login": "chatgpt-codex-connector"}, + } + ], + ) + monkeypatch.setattr(module, "unresolved_review_comment_urls", lambda *_args: {url}) + + nodes = module.pull_review_comment_nodes("Soju06/codex-lb", 714, head_sha="a" * 40) + + assert len(nodes) == 1 + assert nodes[0]["url"] == url