diff --git a/.github/scripts/review_publishing.py b/.github/scripts/review_publishing.py index 1eb42f3..c50bc6a 100644 --- a/.github/scripts/review_publishing.py +++ b/.github/scripts/review_publishing.py @@ -30,6 +30,41 @@ MAX_HISTORY = 20 PROTECTED_DOC_SYNC_HEADS = {"main", "develop"} +REVIEW_THREADS_QUERY = """ +query WhyLogReviewThreads( + $owner: String! + $name: String! + $number: Int! + $after: String +) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $after) { + nodes { + id + isResolved + viewerCanResolve + comments(first: 1) { + nodes { id } + } + } + pageInfo { + hasNextPage + endCursor + } + } + } + } +} +""" + +RESOLVE_REVIEW_THREAD_MUTATION = """ +mutation WhyLogResolveReviewThread($threadId: ID!) { + resolveReviewThread(input: {threadId: $threadId}) { + thread { id isResolved } + } +} +""" class GithubRequest(Protocol): def __call__( @@ -68,6 +103,13 @@ class InlinePublishResult: post_failed_fallback: str | None = None +@dataclass(frozen=True) +class ReviewThreadState: + id: str + is_resolved: bool + viewer_can_resolve: bool + + @dataclass(frozen=True) class DocumentSyncResult: mode: str @@ -326,14 +368,155 @@ def _is_status_error(error: Exception, status: int) -> bool: return getattr(error, "status", None) == status or f"HTTP {status}" in str(error) -def _inline_comment_matches_current_location( - existing: Mapping[str, Any], planned: InlineComment, commit_id: str +def _github_graphql( + api_url: str, + token: str, + query: str, + variables: Mapping[str, Any], + github_request: GithubRequest, +) -> Mapping[str, Any]: + response = github_request( + api_url, + token, + "/graphql", + method="POST", + payload={"query": query, "variables": dict(variables)}, + ) + if not isinstance(response, Mapping): + raise ValueError("GitHub GraphQL returned an invalid response") + errors = response.get("errors") + if errors: + raise ValueError(f"GitHub GraphQL returned errors: {_one_line(errors, 500)}") + data = response.get("data") + if not isinstance(data, Mapping): + raise ValueError("GitHub GraphQL response did not contain data") + return data + + +def _list_review_thread_states( + api_url: str, + token: str, + repository: str, + pr_number: int, + github_request: GithubRequest, +) -> dict[str, ReviewThreadState]: + owner, separator, name = repository.partition("/") + if not separator or not owner or not name or "/" in name: + raise ValueError("repository must use the owner/name format") + + states: dict[str, ReviewThreadState] = {} + cursor: str | None = None + for _ in range(10): + data = _github_graphql( + api_url, + token, + REVIEW_THREADS_QUERY, + { + "owner": owner, + "name": name, + "number": pr_number, + "after": cursor, + }, + github_request, + ) + repository_data = data.get("repository") + pull_request = ( + repository_data.get("pullRequest") + if isinstance(repository_data, Mapping) + else None + ) + threads = ( + pull_request.get("reviewThreads") + if isinstance(pull_request, Mapping) + else None + ) + if not isinstance(threads, Mapping): + raise ValueError("GitHub GraphQL did not return pull request review threads") + + nodes = threads.get("nodes") + if not isinstance(nodes, list): + raise ValueError("GitHub GraphQL returned invalid review thread nodes") + for node in nodes: + if not isinstance(node, Mapping): + continue + thread_id = _string(node.get("id")) + comments = node.get("comments") + comment_nodes = ( + comments.get("nodes") if isinstance(comments, Mapping) else None + ) + if not thread_id or not isinstance(comment_nodes, list): + continue + state = ReviewThreadState( + id=thread_id, + is_resolved=node.get("isResolved") is True, + viewer_can_resolve=node.get("viewerCanResolve") is True, + ) + for comment in comment_nodes: + if not isinstance(comment, Mapping): + continue + comment_node_id = _string(comment.get("id")) + if comment_node_id: + states[comment_node_id] = state + + page_info = threads.get("pageInfo") + if not isinstance(page_info, Mapping): + raise ValueError("GitHub GraphQL returned invalid review thread pagination") + if page_info.get("hasNextPage") is not True: + return states + cursor = _string(page_info.get("endCursor")) or None + if cursor is None: + raise ValueError("GitHub GraphQL omitted the next review thread cursor") + + raise ValueError("GitHub GraphQL review thread pagination exceeded 10 pages") + + +def _resolve_review_thread( + api_url: str, + token: str, + state: ReviewThreadState, + github_request: GithubRequest, ) -> bool: - return ( - _string(existing.get("commit_id")) == commit_id - and _string(existing.get("path")) == planned.path - and _int(existing.get("line")) == planned.line - and _string(existing.get("side")) == "RIGHT" + if state.is_resolved: + return False + if not state.viewer_can_resolve: + raise PermissionError("GitHub token cannot resolve this review thread") + + data = _github_graphql( + api_url, + token, + RESOLVE_REVIEW_THREAD_MUTATION, + {"threadId": state.id}, + github_request, + ) + result = data.get("resolveReviewThread") + thread = result.get("thread") if isinstance(result, Mapping) else None + if ( + not isinstance(thread, Mapping) + or _string(thread.get("id")) != state.id + or thread.get("isResolved") is not True + ): + raise ValueError("GitHub GraphQL did not resolveReviewThread as requested") + return True + + +def _resolve_inline_comment_thread( + api_url: str, + token: str, + comment: Mapping[str, Any], + thread_states: Mapping[str, ReviewThreadState], + github_request: GithubRequest, +) -> bool: + comment_node_id = _string(comment.get("node_id")) + if not comment_node_id: + raise ValueError("GitHub review comment did not contain a node_id") + state = thread_states.get(comment_node_id) + if state is None: + raise ValueError("GitHub GraphQL did not return the review comment thread") + return _resolve_review_thread( + api_url, + token, + state, + github_request, ) @@ -352,87 +535,37 @@ def publish_inline_review_comments( api_url, token, repository, pr_number, github_request ) current = {comment.fingerprint: comment for comment in plan.comments} - updated = 0 resolved = 0 - new_comments: list[InlineComment] = [] - - for fingerprint, comment in current.items(): - matching_comments = [ - item - for item in existing.get(fingerprint, []) - if item.get("id") is not None - and _inline_comment_matches_current_location(item, comment, commit_id) - ] - replaced_comments = [ - item - for item in existing.get(fingerprint, []) - if item.get("id") is not None and item not in matching_comments - ] - for replaced in replaced_comments: - github_request( - api_url, - token, - f"/repos/{repository}/pulls/comments/{replaced['id']}", - method="PATCH", - payload={ - "body": ( - "\n" - "새 커밋의 같은 위치에 최신 자동 리뷰를 다시 등록했습니다." - ) - }, - ) - resolved += 1 - - matching_comments.sort(key=lambda item: int(item["id"])) - if matching_comments: - old = matching_comments[-1] - github_request( + new_comments = [ + comment + for fingerprint, comment in current.items() + if not any( + item.get("id") is not None for item in existing.get(fingerprint, []) + ) + ] + stale_comments = [ + old + for fingerprint, old_comments in existing.items() + if fingerprint not in current + for old in old_comments + if old.get("id") is not None + ] + if stale_comments: + thread_states = _list_review_thread_states( + api_url, token, repository, pr_number, github_request + ) + for old in stale_comments: + if _resolve_inline_comment_thread( api_url, token, - f"/repos/{repository}/pulls/comments/{old['id']}", - method="PATCH", - payload={"body": comment.body}, - ) - updated += 1 - for duplicate in matching_comments[:-1]: - github_request( - api_url, - token, - f"/repos/{repository}/pulls/comments/{duplicate['id']}", - method="PATCH", - payload={ - "body": ( - "\n" - "동일 위치의 중복 자동 리뷰를 최신 코멘트로 통합했습니다." - ) - }, - ) + old, + thread_states, + github_request, + ): resolved += 1 - else: - new_comments.append(comment) - - for fingerprint, old_comments in existing.items(): - if fingerprint in current: - continue - for old in old_comments: - if old.get("id") is None: - continue - body = ( - "\n" - "현재 실행에서 재검출되지 않음(자동 추정). " - "사람이 실제 반영 여부를 확인하세요." - ) - github_request( - api_url, - token, - f"/repos/{repository}/pulls/comments/{old['id']}", - method="PATCH", - payload={"body": body}, - ) - resolved += 1 if not new_comments: - return InlinePublishResult(False, 0, updated, resolved, plan.fallback_findings) + return InlinePublishResult(False, 0, 0, resolved, plan.fallback_findings) posted = 0 fallback = list(plan.fallback_findings) @@ -471,7 +604,7 @@ def publish_inline_review_comments( return InlinePublishResult( posted > 0, posted, - updated, + 0, resolved, tuple(fallback), first_error, diff --git a/.github/scripts/test_review_publishing.py b/.github/scripts/test_review_publishing.py index a619e5a..11e57e4 100644 --- a/.github/scripts/test_review_publishing.py +++ b/.github/scripts/test_review_publishing.py @@ -66,6 +66,47 @@ def __init__(self, status): super().__init__(f"HTTP {status}") +def review_threads_response(*threads: tuple[int, bool]) -> dict: + return { + "data": { + "repository": { + "pullRequest": { + "reviewThreads": { + "nodes": [ + { + "id": f"THREAD_{comment_id}", + "isResolved": resolved, + "viewerCanResolve": not resolved, + "comments": { + "nodes": [{"id": f"COMMENT_{comment_id}"}] + }, + } + for comment_id, resolved in threads + ], + "pageInfo": { + "hasNextPage": False, + "endCursor": None, + }, + } + } + } + } + } + + +def review_thread_mutation_response(comment_id: int) -> dict: + return { + "data": { + "resolveReviewThread": { + "thread": { + "id": f"THREAD_{comment_id}", + "isResolved": True, + } + } + } + } + + class PatchParsingTest(unittest.TestCase): def test_parses_right_side_patch_lines(self): parsed = rp.parse_right_side_lines(files()) @@ -121,7 +162,7 @@ def test_inline_fingerprint_is_stable_for_same_location_when_text_changes(self): class InlinePublishTest(unittest.TestCase): - def test_updates_duplicate_and_marks_stale_resolved(self): + def test_keeps_current_comment_and_resolves_stale_without_rewriting(self): current = finding("block", "server/src/App.java", 2) fingerprint = ( rp.build_inline_review_plan(files(), result(blocking=(current,))) @@ -133,6 +174,7 @@ def test_updates_duplicate_and_marks_stale_resolved(self): [ { "id": 10, + "node_id": "COMMENT_10", "body": f" old", "user": {"type": "Bot"}, "commit_id": "abc", @@ -142,12 +184,13 @@ def test_updates_duplicate_and_marks_stale_resolved(self): }, { "id": 11, + "node_id": "COMMENT_11", "body": " old", "user": {"type": "Bot"}, }, ], - {"id": 10}, - {"id": 11}, + review_threads_response((10, False), (11, False)), + review_thread_mutation_response(11), ] ) @@ -163,10 +206,11 @@ def test_updates_duplicate_and_marks_stale_resolved(self): ) self.assertEqual(published.posted, 0) - self.assertEqual(published.updated, 1) + self.assertEqual(published.updated, 0) self.assertEqual(published.resolved, 1) - self.assertIn("/pulls/comments/10", github.calls[1][0]) - self.assertIn("재검출되지 않음", github.calls[2][2]["body"]) + self.assertEqual(github.calls[1][0], "/graphql") + self.assertIn("resolveReviewThread", github.calls[2][2]["query"]) + self.assertFalse(any(call[1] == "PATCH" for call in github.calls)) def test_posts_new_comments_individually_without_review_wrapper(self): github = FakeGitHub(responses=[[], {"id": 1}]) @@ -195,7 +239,7 @@ def test_posts_new_comments_individually_without_review_wrapper(self): self.assertEqual(github.calls[-1][2]["line"], 2) self.assertEqual(github.calls[-1][2]["commit_id"], "abc") - def test_duplicate_active_comments_are_consolidated(self): + def test_existing_active_comments_are_left_unchanged(self): current = finding("block", "server/src/App.java", 2) fingerprint = ( rp.build_inline_review_plan(files(), result(blocking=(current,))) @@ -207,6 +251,7 @@ def test_duplicate_active_comments_are_consolidated(self): [ { "id": 10, + "node_id": "COMMENT_10", "body": f" old", "user": {"type": "Bot"}, "commit_id": "abc", @@ -216,6 +261,7 @@ def test_duplicate_active_comments_are_consolidated(self): }, { "id": 11, + "node_id": "COMMENT_11", "body": f" duplicate", "user": {"type": "Bot"}, "commit_id": "abc", @@ -223,9 +269,7 @@ def test_duplicate_active_comments_are_consolidated(self): "line": 2, "side": "RIGHT", }, - ], - {"id": 11}, - {"id": 10}, + ] ] ) @@ -240,13 +284,12 @@ def test_duplicate_active_comments_are_consolidated(self): github, ) - self.assertEqual(published.updated, 1) - self.assertEqual(published.resolved, 1) - self.assertIn("/pulls/comments/11", github.calls[1][0]) - self.assertIn("/pulls/comments/10", github.calls[2][0]) - self.assertIn("중복", github.calls[2][2]["body"]) + self.assertEqual(published.posted, 0) + self.assertEqual(published.updated, 0) + self.assertEqual(published.resolved, 0) + self.assertEqual(len(github.calls), 1) - def test_old_commit_comment_is_replaced_on_current_commit(self): + def test_existing_finding_is_never_reopened_or_rewritten(self): current = finding("block", "server/src/App.java", 2) fingerprint = ( rp.build_inline_review_plan(files(), result(blocking=(current,))) @@ -258,16 +301,131 @@ def test_old_commit_comment_is_replaced_on_current_commit(self): [ { "id": 10, + "node_id": "COMMENT_10", "body": f" old", "user": {"type": "Bot"}, - "commit_id": "old-commit", + "commit_id": "abc", "path": "server/src/App.java", "line": 2, "side": "RIGHT", } + ] + ] + ) + + published = rp.publish_inline_review_comments( + "api", + "token", + "WhyLog-App/WhyLog", + 7, + "abc", + files(), + result(blocking=(current,)), + github, + ) + + self.assertEqual(published.posted, 0) + self.assertEqual(published.updated, 0) + self.assertEqual(published.resolved, 0) + self.assertEqual(len(github.calls), 1) + + def test_already_resolved_stale_comment_is_left_unchanged(self): + github = FakeGitHub( + responses=[ + [ + { + "id": 10, + "node_id": "COMMENT_10", + "body": " old", + "user": {"type": "Bot"}, + } ], - {"id": 10}, - {"id": 20}, + review_threads_response((10, True)), + ] + ) + + published = rp.publish_inline_review_comments( + "api", + "token", + "WhyLog-App/WhyLog", + 7, + "abc", + files(), + result(), + github, + ) + + self.assertEqual(published.resolved, 0) + self.assertEqual( + [call[0] for call in github.calls], + [ + "/repos/WhyLog-App/WhyLog/pulls/7/comments?per_page=100&page=1", + "/graphql", + ], + ) + self.assertFalse(any(call[1] == "PATCH" for call in github.calls)) + + def test_resolution_permission_failure_does_not_rewrite_comment(self): + threads = review_threads_response((10, False)) + thread = threads["data"]["repository"]["pullRequest"]["reviewThreads"][ + "nodes" + ][0] + thread["viewerCanResolve"] = False + github = FakeGitHub( + responses=[ + [ + { + "id": 10, + "node_id": "COMMENT_10", + "body": " old", + "user": {"type": "Bot"}, + } + ], + threads, + ] + ) + + with self.assertRaises(PermissionError): + rp.publish_inline_review_comments( + "api", + "token", + "WhyLog-App/WhyLog", + 7, + "abc", + files(), + result(), + github, + ) + + self.assertEqual( + [call[0] for call in github.calls], + [ + "/repos/WhyLog-App/WhyLog/pulls/7/comments?per_page=100&page=1", + "/graphql", + ], + ) + + def test_old_commit_comment_is_not_reposted_while_finding_remains(self): + current = finding("block", "server/src/App.java", 2) + fingerprint = ( + rp.build_inline_review_plan(files(), result(blocking=(current,))) + .comments[0] + .fingerprint + ) + github = FakeGitHub( + responses=[ + [ + { + "id": 10, + "node_id": "COMMENT_10", + "body": f" old", + "user": {"type": "Bot"}, + "commit_id": "old-commit", + "path": "server/src/App.java", + "line": 2, + "side": "RIGHT", + } + ] ] ) @@ -282,12 +440,10 @@ def test_old_commit_comment_is_replaced_on_current_commit(self): github, ) - self.assertEqual(published.posted, 1) + self.assertEqual(published.posted, 0) self.assertEqual(published.updated, 0) - self.assertEqual(published.resolved, 1) - self.assertIn("최신 자동 리뷰", github.calls[1][2]["body"]) - self.assertEqual(github.calls[2][1], "POST") - self.assertEqual(github.calls[2][2]["commit_id"], "new-commit") + self.assertEqual(published.resolved, 0) + self.assertEqual(len(github.calls), 1) def test_one_rejected_comment_does_not_drop_other_inline_comments(self): github = FakeGitHub( @@ -595,6 +751,8 @@ def test_sync_creates_commit_from_reviewed_head(self): commit_call[2]["message"], "docs(docs): PR-7 리뷰 판단 근거 기록", ) + self.assertNotIn("author", commit_call[2]) + self.assertNotIn("committer", commit_call[2]) def test_sync_stale_head_is_artifact_only(self): github = FakeGitHub( diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da49bbc..14d40a2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -218,7 +218,7 @@ jobs: env: GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} - AI_REVIEW_PUSH_TOKEN: "" + AI_REVIEW_PUSH_TOKEN: ${{ secrets.AI_REVIEW_PUSH_TOKEN }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPOSITORY_IS_PRIVATE: ${{ github.event.repository.private }} PYTHONUNBUFFERED: "1" diff --git a/docs/pr-reviews/PR-8.md b/docs/pr-reviews/PR-8.md new file mode 100644 index 0000000..1a80b43 --- /dev/null +++ b/docs/pr-reviews/PR-8.md @@ -0,0 +1,36 @@ +# PR-8 AI 리뷰 기록 + +- PR: https://github.com/WhyLog-App/WhyLog/pull/8 +- 제목: ci(root): AI 리뷰 게시 흐름 검증 +- 브랜치: `develop` ← `ci/ai-review-publishing` +- HEAD: `d90c93b6559e6fea4131b314047810403005e248` +- 입력 digest: `47fbb8ed4ce3cc065bc431c019f4fe6f725fb4428a1d946071a3b92bdd0e4754` +- 모델: Google `gemini-3.6-flash` +- 상태: **PASS** +- 생성 시각(UTC): 2026-08-09T15:09:19+00:00 + +## 요약 + +CI 리뷰 스크립트(.github/scripts/review_publishing.py)에서 GitHub GraphQL API를 사용하여 재검출되지 않은 인라인 리뷰 스레드를 자동으로 resolve 처리하는 로직 추가 및 워크플로우(ci.yml) 토큰 설정 수정입니다. 변경 사항이 규칙 및 기존 계약을 준수하며 테스트로 잘 검증되어 있습니다. + +## 이번 실행에서 새로 발견됨 + +없음 + +## 이전 실행부터 계속 남아있음 + +없음 + +## 현재까지 사라짐(자동 추정) + +없음 + +## 실행 이력 + +|HEAD|상태|모델|전체|신규|계속|해결|시각| +|---|---|---|---:|---:|---:|---:|---| +|d90c93b6559e|PASS|Google gemini-3.6-flash|||||2026-08-09T15:09:19+00:00| +|38a51157fa78|PASS|Google gemini-3.6-flash|||||2026-08-09T15:01:31+00:00| + + +